Skip to content

Conversation

@marcpaterno
Copy link
Collaborator

@marcpaterno marcpaterno commented Jan 27, 2026

Description

This PR consolidates the ccl_factory module into modeling_tools and moves the parameters module into updatable to improve code organization and better reflect the architectural relationships between these components.

The CCL factory functionality is closely related to modeling tools and now resides within the same module. Similarly, parameter-related classes are now part of the updatable module, reflecting their tight coupling with updatable objects.

Both old modules (firecrown.ccl_factory and firecrown.parameters) remain as deprecated compatibility layers that re-export from their new locations with deprecation warnings, ensuring backward compatibility for existing code.

All imports throughout the codebase (examples, tests, connectors, and tutorials) have been updated to use the new module paths.

Type of Change

Refactoring, with backward compatibility support

Checklist:

The following checklist will make sure that you are following the code style and
guidelines of the project as described in the
contributing page.

[x] I have run bash pre-commit-check and fixed any issues
[x] I have added tests that prove my fix is effective or that my feature works
[x] I have made corresponding changes to the documentation
[ ] I have 100% test coverage for my changes (please check this after the CI system has verified the coverage)

marcpaterno and others added 6 commits January 12, 2026 15:20
* Introduce BinPairSelector class system for bin combinations.
* Reorganize all tutorials.
* New tutorial for Selectors.
---------
Co-authored-by: Marc Paterno <[email protected]>
Consolidate the ccl_factory module into modeling_tools to improve
code organization. The CCL factory functionality is closely related
to the modeling tools and belongs in the same module.

Changes:
- Move ccl_factory submodules into modeling_tools as _ccl_* files
- Update modeling_tools __init__ to export CCL factory classes
- Replace ccl_factory with deprecation wrapper that re-exports
- Update all imports across examples, tests, and connectors
- Add deprecation warning when ccl_factory is imported
- Add tests for deprecated module backward compatibility
- Rename test_ccl_factory.py to test_modeling_tools_ccl_factory.py
Correct the module path reference in the two-point
integration tutorial from
`firecrown.generators.inferred_galaxy_zdist` to
`firecrown.generators`, which is the accurate import
location for the lens and source redshift bin collections.
The firecrown.parameters module has been moved into
firecrown.updatable to better reflect the architectural
relationship between parameters and updatable objects.

All parameter-related classes (DerivedParameter,
DerivedParameterCollection, InternalParameter, ParamsMap,
RequiredParameters, SamplerParameter) and functions
(handle_unused_params, parameter_get_full_name,
register_new_updatable_parameter) are now exported from
firecrown.updatable.

The old firecrown.parameters module remains as a deprecated
compatibility layer that re-exports from firecrown.updatable
with deprecation warnings.

All imports throughout the codebase have been updated to use
firecrown.updatable instead of firecrown.parameters.

Tests have been added to verify the deprecated module
continues to work correctly with appropriate warnings, and
existing parameter tests have been renamed to reflect their
new location.
Copy link

Copilot AI left a 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 is a significant refactoring pull request titled "Fold packages" that reorganizes Firecrown's module structure and adds extensive tutorial documentation. The PR includes:

Changes:

  • Module reorganization: Moves parameter-related classes from firecrown.parameters to firecrown.updatable, and CCL factory classes from firecrown.ccl_factory to firecrown.modeling_tools
  • Deprecation wrappers: Maintains backward compatibility through deprecated modules with proper warnings
  • New tutorial structure: Adds 6 new comprehensive tutorial files for two-point statistics workflows
  • Bin pair selectors: Introduces a powerful new filtering system for tomographic bin pair selection
  • Generator naming: Updates LSST bin naming convention to be more systematic

Reviewed changes

Copilot reviewed 114 out of 118 changed files in this pull request and generated 12 comments.

Show a summary per file
File Description
tutorial/*.qmd New comprehensive tutorial documentation
firecrown/parameters/init.py Deprecation wrapper maintaining backward compatibility
firecrown/ccl_factory/init.py Deprecation wrapper maintaining backward compatibility
firecrown/updatable/* Reorganized parameter-related functionality
firecrown/modeling_tools/* Reorganized CCL factory functionality
firecrown/metadata_types/_pair_selector.py New bin pair selector system (780 lines)
tests/* Updated imports throughout test suite
examples/* Updated imports in example scripts

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

match="firecrown.ccl_factory is deprecated and will be removed",
):
# pylint: disable=import-outside-toplevel,unused-import
import firecrown.ccl_factory # noqa: F401
Copy link

Copilot AI Jan 27, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Module 'firecrown.ccl_factory' is imported with both 'import' and 'import from'.

Copilot uses AI. Check for mistakes.
We just verify the module is importable.
"""
# pylint: disable=import-outside-toplevel,unused-import
import firecrown.ccl_factory # noqa: F401
Copy link

Copilot AI Jan 27, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Module 'firecrown.ccl_factory' is imported with both 'import' and 'import from'.

Copilot uses AI. Check for mistakes.
def test_module_import_as_alias():
"""Test importing the module with an alias."""
# pylint: disable=import-outside-toplevel
import firecrown.ccl_factory as fac
Copy link

Copilot AI Jan 27, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Module 'firecrown.ccl_factory' is imported with both 'import' and 'import from'.

Copilot uses AI. Check for mistakes.
def test_ccl_factory_all_list():
"""Test that __all__ is preserved in deprecated module."""
# pylint: disable=import-outside-toplevel
import firecrown.ccl_factory
Copy link

Copilot AI Jan 27, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Module 'firecrown.ccl_factory' is imported with both 'import' and 'import from'.

Copilot uses AI. Check for mistakes.
import sacc

import firecrown.parameters
import firecrown.updatable
Copy link

Copilot AI Jan 27, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Module 'firecrown.updatable' is imported with both 'import' and 'import from'.

Copilot uses AI. Check for mistakes.
warnings.simplefilter("always")
# Import should trigger warning
# pylint: disable=unused-import,import-outside-toplevel
import firecrown.parameters # noqa: F401
Copy link

Copilot AI Jan 27, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Module 'firecrown.parameters' is imported with both 'import' and 'import from'.

Copilot uses AI. Check for mistakes.
def test_parameters_exports_all_names():
"""All expected names should be available from firecrown.parameters."""
# pylint: disable=import-outside-toplevel
import firecrown.parameters as params
Copy link

Copilot AI Jan 27, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Module 'firecrown.parameters' is imported with both 'import' and 'import from'.

Copilot uses AI. Check for mistakes.
def test_parameters_exports_are_same_as_updatable():
"""Exported names should be the same objects as in firecrown.updatable."""
# pylint: disable=import-outside-toplevel
import firecrown.parameters as params
Copy link

Copilot AI Jan 27, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Module 'firecrown.parameters' is imported with both 'import' and 'import from'.

Copilot uses AI. Check for mistakes.
from firecrown.ccl_factory import CCLFactory, PoweSpecAmplitudeParameter
import firecrown.parameters as fcp
from firecrown.modeling_tools import CCLFactory, PoweSpecAmplitudeParameter
import firecrown.updatable as fcp
Copy link

Copilot AI Jan 27, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Module 'firecrown.updatable' is imported with both 'import' and 'import from'.

Copilot uses AI. Check for mistakes.
import sacc

import firecrown.parameters
import firecrown.updatable
Copy link

Copilot AI Jan 27, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Module 'firecrown.updatable' is imported with both 'import' and 'import from'.

Copilot uses AI. Check for mistakes.
@codecov
Copy link

codecov bot commented Jan 27, 2026

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 100.0%. Comparing base (de854f0) to head (f55f3c2).
⚠️ Report is 2 commits behind head on devel.

Additional details and impacted files
@@           Coverage Diff            @@
##            devel     #610    +/-   ##
========================================
  Coverage   100.0%   100.0%            
========================================
  Files         150      151     +1     
  Lines        8787     9044   +257     
  Branches     1012     1033    +21     
========================================
+ Hits         8787     9044   +257     
Files with missing lines Coverage Δ
firecrown/app/analysis/_types.py 100.0% <100.0%> (ø)
firecrown/app/cosmology.py 100.0% <100.0%> (ø)
firecrown/app/examples/_cosmic_shear_template.py 100.0% <100.0%> (ø)
...irecrown/app/examples/_des_y1_3x2pt_pt_template.py 100.0% <100.0%> (ø)
firecrown/app/examples/_des_y1_3x2pt_template.py 100.0% <100.0%> (ø)
...app/examples/_des_y1_cosmic_shear_hmia_template.py 100.0% <100.0%> (ø)
...mples/_des_y1_cosmic_shear_pk_modifier_template.py 100.0% <100.0%> (ø)
...app/examples/_des_y1_cosmic_shear_tatt_template.py 100.0% <100.0%> (ø)
firecrown/app/examples/_sn_srd_template.py 100.0% <100.0%> (ø)
firecrown/ccl_factory/__init__.py 100.0% <100.0%> (ø)
... and 44 more
🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@marcpaterno
Copy link
Collaborator Author

Close because of confusion with earlier commits. Will create a new PR.

@marcpaterno marcpaterno deleted the fold_packages branch January 27, 2026 19:26
@marcpaterno marcpaterno restored the fold_packages branch January 27, 2026 19:26
@marcpaterno marcpaterno deleted the fold_packages branch January 27, 2026 23:03
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.

3 participants