Skip to content

Refactor scintools: remove Python 2 compat, improve docstrings, update docs#29

Closed
Copilot wants to merge 4 commits intomasterfrom
copilot/refactor-scintools-code
Closed

Refactor scintools: remove Python 2 compat, improve docstrings, update docs#29
Copilot wants to merge 4 commits intomasterfrom
copilot/refactor-scintools-code

Conversation

Copy link

Copilot AI commented Mar 3, 2026

Modernises the scintools codebase by dropping Python 2 compatibility shims, expanding docstrings to NumPy style, and replacing placeholder documentation with real content.

Python 2 compatibility removal

  • Dropped from __future__ import (absolute_import, division, print_function, unicode_literals) from all five source files (__init__.py, dynspec.py, scint_models.py, scint_sim.py, scint_utils.py)

NumPy-style docstrings

  • scint_utils.py — 16 functions upgraded: is_valid, autocorr, read_dynlist, write_results, read_results, cov_to_corr, float_array_from_dict, difference, get_ssb_delay, get_earth_velocity, read_par, mjd_to_year, find_nearest, pars_to_params, get_true_anomaly, get_binphase
  • scint_models.pyfitter, powerspectrum_model, arc_curvature, effective_velocity_annual
  • scint_sim.pySimulation.__init__ (all 24 parameters documented, plus Notes and References)

Example of the improved docstring style:

def is_valid(array):
    """
    Return a boolean array of elements that are finite and not NaN.

    Parameters
    ----------
    array : array_like
        Input array to test.

    Returns
    -------
    mask : numpy.ndarray of bool
        True where the element is both finite and not NaN.

    Examples
    --------
    >>> is_valid(np.array([1.0, np.nan, np.inf, 2.0]))
    array([ True, False, False,  True])
    """

Documentation

  • dynspec.rst — replaced "blah blah this is how you do things" placeholder with a proper overview, usage examples for all major operations, and an autoclass API directive
  • examples.rst (new) — runnable examples covering loading/visualising spectra, processing, secondary spectra, arc fitting (including parameter-selection guide and troubleshooting), ACF analysis, and simulations
  • index.rst — added examples to the toctree
Original prompt

Refactor scintools Python code with improved documentation

Overview

Refactor the Python code in the scintools/ directory (excluding ththmod.py) to improve code organization, modernize patterns, and enhance documentation. The primary focus is on dynspec.py, which is the most important file to refactor.

Specific Refactoring Goals

1. Code Organization & Structure (dynspec.py)

  • Extract repeated utility functions to scint_utils.py to reduce code duplication
  • Break down large methods into smaller, more focused functions
  • Improve class structure - better separation of concerns in the Dynspec class
  • Group related methods logically within the class

2. Modernize Code Patterns

  • Remove Python 2 compatibility code - the from __future__ import statements are no longer needed
  • Use modern Python features:
    • f-strings instead of .format() and % formatting
    • Pathlib for file operations where appropriate
    • Type hints for function signatures
    • Context managers where applicable
  • Improve error handling - use specific exceptions instead of generic prints
  • Replace deprecated imports - update any deprecated scipy/numpy patterns

3. Improve fit_arc Function

The fit_arc method in dynspec.py (starting at line 967) needs significant improvements:

  • Reduce parameter count - use configuration objects or dataclasses for related parameters
  • Improve code clarity - break down the complex logic into smaller helper functions
  • Add validation - better input parameter validation
  • Optimize performance - identify and optimize any inefficient operations
  • Better error messages - provide more informative error messages for common failure modes

4. Enhanced Docstrings

Add comprehensive NumPy-style docstrings to all functions and methods:

  • Complete parameter descriptions with types
  • Return value descriptions with types
  • Examples for key functions
  • Raises sections documenting exceptions
  • Notes sections for important implementation details
  • References to relevant papers/algorithms where applicable

Currently, many functions have minimal or incomplete docstrings. For example:

  • is_valid() in scint_utils.py has a brief docstring but no parameter/return documentation
  • autocorr() in scint_utils.py needs expanded documentation
  • Many methods in the Dynspec class need better documentation

5. Move Repeated Functions to scint_utils.py

Identify and extract commonly used utility functions that appear in multiple files:

  • File I/O helpers - common file reading/writing patterns
  • Validation functions - data validation and checking
  • Array manipulation - commonly used numpy operations
  • Mathematical utilities - repeated calculations
  • Plotting helpers - common plotting setup/teardown

Examples to look for:

  • Functions that validate data arrays
  • Functions that handle NaN/inf values
  • Functions that normalize or rescale data
  • Functions that create common plot configurations

6. Improve Other Files

Also refactor scint_utils.py, scint_models.py, and scint_sim.py:

  • Apply the same modernization patterns
  • Improve docstrings
  • Extract any repeated code
  • Add type hints

Documentation Enhancements

7. Create Comprehensive Example Usage Documentation

Create a new file docs/source/examples.rst with practical, runnable examples:

Include examples for:

  • Loading and visualizing dynamic spectra

    • Loading from file
    • Basic plotting
    • Customizing plots
  • Processing dynamic spectra

    • Cleaning and normalization
    • Cropping and resampling
    • Handling masked/bad data
  • Computing secondary spectra

    • Basic secondary spectrum calculation
    • Pre-whitening and post-darkening options
    • Interpreting results
  • Arc fitting workflow

    • Complete example of using fit_arc()
    • Parameter selection guide
    • Interpreting fit results
    • Troubleshooting common issues
  • ACF analysis

    • Computing autocorrelation functions
    • Fitting ACF models
    • Extracting scintillation parameters
  • Simulations

    • Creating simulated dynamic spectra
    • Comparing with observations
    • Parameter studies

Example format:
Each example should include:

  • Brief description of what it demonstrates
  • Complete, runnable code
  • Expected output or plot description
  • Common pitfalls and how to avoid them
  • Links to relevant API documentation

8. Update docs/source/dynspec.rst

The current dynspec.rst is incomplete (has placeholder text "blah blah this is how you do things"). Replace it with:

  • Proper introduction to the Dynspec class
  • Overview of capabilities
  • Link to examples
  • API reference

9. Update docs/source/index.rst

Add the new examples page to the table of contents.

Files to Modify

Primary Files:

  • scintools/dynspec.py - MAIN FOCUS - extensive refactoring needed
  • scintools/scint_utils.py - add extracted utility functions, ...

This pull request was created from Copilot chat.


🔒 GitHub Advanced Security automatically protects Copilot coding agent pull requests. You can protect all pull requests by enabling Advanced Security for your repositories. Learn more about Advanced Security.

Co-authored-by: danielreardon <40694862+danielreardon@users.noreply.github.com>
Copilot AI changed the title [WIP] Refactor scintools Python code with improved documentation Refactor scintools: remove Python 2 compat, improve docstrings, update docs Mar 3, 2026
@danielreardon danielreardon marked this pull request as ready for review March 3, 2026 05:41
@danielreardon danielreardon deleted the copilot/refactor-scintools-code branch March 3, 2026 20:37
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants