-
Notifications
You must be signed in to change notification settings - Fork 3
Packaging improvements #92
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull request overview
This pull request focuses on improving the code quality and documentation of the flip package through linting fixes, comprehensive docstring generation, and structural improvements.
Key Changes
- Replaced bare
except:with specificexcept ImportError:for better error handling - Renamed
config.pyto_config.pyand updated all imports accordingly - Added comprehensive Google-style docstrings across all modules
- Improved variable naming (e.g.,
l→ellfor mathematical clarity) - Replaced
eval()calls with saferimportlibandgetattr()patterns - Fixed type comparisons from
type(x) == type(None)tox is not None
Reviewed changes
Copilot reviewed 34 out of 35 changed files in this pull request and generated 5 comments.
Show a summary per file
| File | Description |
|---|---|
| flip/utils.py | Added docstrings for utility functions |
| flip/likelihood.py | Import path fix + comprehensive class/method documentation |
| flip/power_spectra/*.py | Exception handling improvements + docstrings |
| flip/covariance/*.py | Import path updates, variable renaming (l→ell), docstrings |
| flip/fitter.py | Reorganized imports + added docstrings |
| flip/data_vector/*.py | Import path updates + docstrings |
| flip/_config.py | New configuration file (renamed from config.py) |
Comments suppressed due to low confidence (2)
flip/covariance/lai22/symbolic.py:143
- File may not be closed if this operation raises an exception.
File may not be closed if this operation raises an exception.
File may not be closed if this operation raises an exception.
File may not be closed if this operation raises an exception.
File may not be closed if this operation raises an exception.
File may not be closed if this operation raises an exception.
File may not be closed if this operation raises an exception.
File may not be closed if this operation raises an exception.
File may not be closed if this operation raises an exception.
File may not be closed if this operation raises an exception.
File may not be closed if this operation raises an exception.
File may not be closed if this operation raises an exception.
File may not be closed if this operation raises an exception.
File may not be closed if this operation raises an exception.
File may not be closed if this operation raises an exception.
File may not be closed if this operation raises an exception.
File may not be closed if this operation raises an exception.
File may not be closed if this operation raises an exception.
File may not be closed if this operation raises an exception.
f = open(filename, "w")
flip/covariance/symbolic.py:201
- File may not be closed if this operation raises an exception.
File may not be closed if this operation raises an exception.
File may not be closed if this operation raises an exception.
File may not be closed if this operation raises an exception.
File may not be closed if this operation raises an exception.
File may not be closed if this operation raises an exception.
File may not be closed if this operation raises an exception.
File may not be closed if this operation raises an exception.
File may not be closed if this operation raises an exception.
File may not be closed if this operation raises an exception.
File may not be closed if this operation raises an exception.
File may not be closed if this operation raises an exception.
File may not be closed if this operation raises an exception.
File may not be closed if this operation raises an exception.
File may not be closed if this operation raises an exception.
File may not be closed if this operation raises an exception.
File may not be closed if this operation raises an exception.
File may not be closed if this operation raises an exception.
File may not be closed if this operation raises an exception.
File may not be closed if this operation raises an exception.
File may not be closed if this operation raises an exception.
File may not be closed if this operation raises an exception.
File may not be closed if this operation raises an exception.
File may not be closed if this operation raises an exception.
File may not be closed if this operation raises an exception.
File may not be closed if this operation raises an exception.
File may not be closed if this operation raises an exception.
File may not be closed if this operation raises an exception.
File may not be closed if this operation raises an exception.
File may not be closed if this operation raises an exception.
File may not be closed if this operation raises an exception.
File may not be closed if this operation raises an exception.
File may not be closed if this operation raises an exception.
File may not be closed if this operation raises an exception.
File may not be closed if this operation raises an exception.
File may not be closed if this operation raises an exception.
File may not be closed if this operation raises an exception.
File may not be closed if this operation raises an exception.
f = open(filename, "w")
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
flip/power_spectra/generator.py
Outdated
| model_function = importlib.import_module( | ||
| f"flip.power_spectra.models.get_{power_spectrum_model}_model" | ||
| ) |
Copilot
AI
Dec 3, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Incorrect usage of importlib.import_module. This function imports a module, not a function. The code should be:
models = importlib.import_module(f"flip.power_spectra.models")
model_function = getattr(models, f"get_{power_spectrum_model}_model")The current code will fail at runtime with an ImportError since flip.power_spectra.models.get_{power_spectrum_model}_model is not a module path.
| @@ -147,6 +236,7 @@ def covariance_gg( | |||
| size_batch=100_000, | |||
| number_worker=8, | |||
| ): | |||
| """Compute density-density covariance (plane-parallel)." | |||
Copilot
AI
Dec 3, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Missing closing quote in docstring. The line should end with:
"""Compute density-density covariance (plane-parallel)."""| """Compute density-density covariance (plane-parallel)." | |
| """Compute density-density covariance (plane-parallel).""" |
| """Create an emcee sampler with optional HDF backend. | ||
|
|
||
| Args: | ||
| likelihood (Callable): Log-probability function. | ||
| p0 (numpy.ndarray, optional): Initial positions. | ||
| backend_file (str, optional): HDF backend filename to resume/checkpoint. | ||
| """ |
Copilot
AI
Dec 3, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Misplaced docstring. The docstring should be inside the __init__ method, not after the super().__init__() call. Move lines 481-487 to be the first line after the function signature on line 479.
flip/covariance/generator.py
Outdated
| """Evaluate and optionally regularize M(k) term. | ||
|
|
||
| Applies one of the supported regularizations to stabilize numerical | ||
| behavior (mpmath high-precision, Savitzky–Golay smoothing, or low-k | ||
| asymptote detection). | ||
|
|
||
| Args: | ||
| M_function (callable): Function returning M(k) given additional parameters. | ||
| wavenumber (ndarray): k samples. | ||
| regularize_M_terms (dict|None): Per type regularization option. | ||
| covariance_type (str): `"gg"`, `"gv"`, or `"vv"`. | ||
| flip_terms (module): Terms module to switch backend when needed. | ||
| additional_parameters_values (dict|tuple): Extra parameters for M. | ||
| savgol_window (int): Window size for Savitzky–Golay. | ||
| savgol_polynomial (int): Polynomial order for Savitzky–Golay. | ||
| lowk_unstable_threshold (float): Threshold for low-k asymptote detection. | ||
| lowk_unstable_mean_filtering (int): Window for mean filtering indices. | ||
| mpmmath_decimal_precision (int): Decimal precision for mpmath. | ||
|
|
||
| Returns: | ||
| ndarray: Evaluated M(k) after regularization. | ||
| """ |
Copilot
AI
Dec 3, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Misplaced docstring inside function body. The docstring for regularize_M should start on line 239, not line 240. Move the entire docstring block (lines 240-261) to immediately after line 239.
| """Evaluate and optionally regularize M(k) term. | |
| Applies one of the supported regularizations to stabilize numerical | |
| behavior (mpmath high-precision, Savitzky–Golay smoothing, or low-k | |
| asymptote detection). | |
| Args: | |
| M_function (callable): Function returning M(k) given additional parameters. | |
| wavenumber (ndarray): k samples. | |
| regularize_M_terms (dict|None): Per type regularization option. | |
| covariance_type (str): `"gg"`, `"gv"`, or `"vv"`. | |
| flip_terms (module): Terms module to switch backend when needed. | |
| additional_parameters_values (dict|tuple): Extra parameters for M. | |
| savgol_window (int): Window size for Savitzky–Golay. | |
| savgol_polynomial (int): Polynomial order for Savitzky–Golay. | |
| lowk_unstable_threshold (float): Threshold for low-k asymptote detection. | |
| lowk_unstable_mean_filtering (int): Window for mean filtering indices. | |
| mpmmath_decimal_precision (int): Decimal precision for mpmath. | |
| Returns: | |
| ndarray: Evaluated M(k) after regularization. | |
| """ | |
| """Evaluate and optionally regularize M(k) term. | |
| Applies one of the supported regularizations to stabilize numerical | |
| behavior (mpmath high-precision, Savitzky–Golay smoothing, or low-k | |
| asymptote detection). | |
| Args: | |
| M_function (callable): Function returning M(k) given additional parameters. | |
| wavenumber (ndarray): k samples. | |
| regularize_M_terms (dict|None): Per type regularization option. | |
| covariance_type (str): `"gg"`, `"gv"`, or `"vv"`. | |
| flip_terms (module): Terms module to switch backend when needed. | |
| additional_parameters_values (dict|tuple): Extra parameters for M. | |
| savgol_window (int): Window size for Savitzky–Golay. | |
| savgol_polynomial (int): Polynomial order for Savitzky–Golay. | |
| lowk_unstable_threshold (float): Threshold for low-k asymptote detection. | |
| lowk_unstable_mean_filtering (int): Window for mean filtering indices. | |
| mpmmath_decimal_precision (int): Decimal precision for mpmath. | |
| Returns: | |
| ndarray: Evaluated M(k) after regularization. | |
| """ |
| @@ -331,7 +299,7 @@ def run(self, migrad=True, hesse=False, minos=False, n_iter=1): | |||
| if minos: | |||
| try: | |||
| log.add(self.minuit.minos()) | |||
| except: | |||
| except RuntimeError: | |||
Copilot
AI
Dec 3, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
'except' clause does nothing but pass and there is no explanatory comment.
| except RuntimeError: | |
| except RuntimeError: | |
| # minuit.minos() may fail for some parameter configurations; ignore and proceed. |
|
Rather simple tests are working now |
…ld, still small sample, need to rerun the notebooks and tests
|
Tests improved and test pipeline working. Adding a script to refresh the test values |
|
Fisher seems broken (blocked) with JAX at the moment |
…is only for the covariance method
|
New changes:
Changes for future PR:
|
|
According to the extension of package,
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull request overview
Copilot reviewed 76 out of 119 changed files in this pull request and generated 2 comments.
Comments suppressed due to low confidence (3)
flip/covariance/fitter.py:304
- 'except' clause does nothing but pass and there is no explanatory comment.
flip/covariance/analytical/lai22/symbolic.py:143 - File may not be closed if this operation raises an exception.
File may not be closed if this operation raises an exception.
File may not be closed if this operation raises an exception.
File may not be closed if this operation raises an exception.
File may not be closed if this operation raises an exception.
File may not be closed if this operation raises an exception.
File may not be closed if this operation raises an exception.
File may not be closed if this operation raises an exception.
File may not be closed if this operation raises an exception.
File may not be closed if this operation raises an exception.
File may not be closed if this operation raises an exception.
File may not be closed if this operation raises an exception.
File may not be closed if this operation raises an exception.
File may not be closed if this operation raises an exception.
File may not be closed if this operation raises an exception.
File may not be closed if this operation raises an exception.
File may not be closed if this operation raises an exception.
File may not be closed if this operation raises an exception.
File may not be closed if this operation raises an exception.
flip/covariance/symbolic.py:201 - File may not be closed if this operation raises an exception.
File may not be closed if this operation raises an exception.
File may not be closed if this operation raises an exception.
File may not be closed if this operation raises an exception.
File may not be closed if this operation raises an exception.
File may not be closed if this operation raises an exception.
File may not be closed if this operation raises an exception.
File may not be closed if this operation raises an exception.
File may not be closed if this operation raises an exception.
File may not be closed if this operation raises an exception.
File may not be closed if this operation raises an exception.
File may not be closed if this operation raises an exception.
File may not be closed if this operation raises an exception.
File may not be closed if this operation raises an exception.
File may not be closed if this operation raises an exception.
File may not be closed if this operation raises an exception.
File may not be closed if this operation raises an exception.
File may not be closed if this operation raises an exception.
File may not be closed if this operation raises an exception.
File may not be closed if this operation raises an exception.
File may not be closed if this operation raises an exception.
File may not be closed if this operation raises an exception.
File may not be closed if this operation raises an exception.
File may not be closed if this operation raises an exception.
File may not be closed if this operation raises an exception.
File may not be closed if this operation raises an exception.
File may not be closed if this operation raises an exception.
File may not be closed if this operation raises an exception.
File may not be closed if this operation raises an exception.
File may not be closed if this operation raises an exception.
File may not be closed if this operation raises an exception.
File may not be closed if this operation raises an exception.
File may not be closed if this operation raises an exception.
File may not be closed if this operation raises an exception.
File may not be closed if this operation raises an exception.
File may not be closed if this operation raises an exception.
File may not be closed if this operation raises an exception.
File may not be closed if this operation raises an exception.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
flip/data/load_data_test.py
Outdated
| from flip import utils | ||
|
|
||
| (kmm, pmm), (kmt, pmt), (ktt, ptt) = load_power_spectra() | ||
| power_spectrum_dict = { |
Copilot
AI
Jan 23, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This assignment to 'power_spectrum_dict' is unnecessary as it is redefined before this value is used.
| return self._data | ||
|
|
||
| @abc.abstractmethod | ||
| def give_data_and_variance(self, **kwargs): |
Copilot
AI
Jan 23, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Overridden method signature does not match call, where it is passed too many arguments. Overriding method method Dens.give_data_and_variance matches the call.
Overridden method signature does not match call, where it is passed too many arguments. Overriding method method DirectVel.give_data_and_variance matches the call.
Overridden method signature does not match call, where it is passed too many arguments. Overriding method method DirectVel.give_data_and_variance matches the call.
Overridden method signature does not match call, where it is passed too many arguments. Overriding method method Dens.give_data_and_variance matches the call.
Overridden method signature does not match call, where it is passed too many arguments. Overriding method method DirectVel.give_data_and_variance matches the call.
Overridden method signature does not match call, where it is passed too many arguments. Overriding method method DensVel.give_data_and_variance matches the call.
|
This branch is major and will be the basis for v1.2 once the versioning pipeline works |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull request overview
Copilot reviewed 76 out of 119 changed files in this pull request and generated no new comments.
Comments suppressed due to low confidence (3)
flip/covariance/fitter.py:304
- 'except' clause does nothing but pass and there is no explanatory comment.
flip/covariance/analytical/lai22/symbolic.py:143 - File may not be closed if this operation raises an exception.
File may not be closed if this operation raises an exception.
File may not be closed if this operation raises an exception.
File may not be closed if this operation raises an exception.
File may not be closed if this operation raises an exception.
File may not be closed if this operation raises an exception.
File may not be closed if this operation raises an exception.
File may not be closed if this operation raises an exception.
File may not be closed if this operation raises an exception.
File may not be closed if this operation raises an exception.
File may not be closed if this operation raises an exception.
File may not be closed if this operation raises an exception.
File may not be closed if this operation raises an exception.
File may not be closed if this operation raises an exception.
File may not be closed if this operation raises an exception.
File may not be closed if this operation raises an exception.
File may not be closed if this operation raises an exception.
File may not be closed if this operation raises an exception.
File may not be closed if this operation raises an exception.
flip/covariance/symbolic.py:201 - File may not be closed if this operation raises an exception.
File may not be closed if this operation raises an exception.
File may not be closed if this operation raises an exception.
File may not be closed if this operation raises an exception.
File may not be closed if this operation raises an exception.
File may not be closed if this operation raises an exception.
File may not be closed if this operation raises an exception.
File may not be closed if this operation raises an exception.
File may not be closed if this operation raises an exception.
File may not be closed if this operation raises an exception.
File may not be closed if this operation raises an exception.
File may not be closed if this operation raises an exception.
File may not be closed if this operation raises an exception.
File may not be closed if this operation raises an exception.
File may not be closed if this operation raises an exception.
File may not be closed if this operation raises an exception.
File may not be closed if this operation raises an exception.
File may not be closed if this operation raises an exception.
File may not be closed if this operation raises an exception.
File may not be closed if this operation raises an exception.
File may not be closed if this operation raises an exception.
File may not be closed if this operation raises an exception.
File may not be closed if this operation raises an exception.
File may not be closed if this operation raises an exception.
File may not be closed if this operation raises an exception.
File may not be closed if this operation raises an exception.
File may not be closed if this operation raises an exception.
File may not be closed if this operation raises an exception.
File may not be closed if this operation raises an exception.
File may not be closed if this operation raises an exception.
File may not be closed if this operation raises an exception.
File may not be closed if this operation raises an exception.
File may not be closed if this operation raises an exception.
File may not be closed if this operation raises an exception.
File may not be closed if this operation raises an exception.
File may not be closed if this operation raises an exception.
File may not be closed if this operation raises an exception.
File may not be closed if this operation raises an exception.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
A lot of change to improve the readability of flip. For now it includes:
Planned change in this PR: