Skip to content

Conversation

@rerpha
Copy link
Contributor

@rerpha rerpha commented Apr 2, 2025

closes #68

This is a large breaking change.

Main highlights are:

  • moved callbacks\fitting\fitting_utils.py to its own module (fitting.py)
  • exposed all user-facing callbacks in __init__.py of callbacks
  • moved single __init__.pys for modules to the top level, named the same as their modules - though this shouldn;'t make a difference from a user-facing perspective as the import looks the same
  • remove empty modules ie plans{muons, sans}

@rerpha rerpha marked this pull request as ready for review April 2, 2025 15:13
Copy link
Member

@Tom-Willemsen Tom-Willemsen left a comment

Choose a reason for hiding this comment

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

first pass, not done any functional testing yet

@rerpha rerpha requested a review from Tom-Willemsen April 2, 2025 20:09
@Tom-Willemsen
Copy link
Member

Was there a matching PR/branch for system_tests repo? I couldn't see one

@rerpha
Copy link
Contributor Author

rerpha commented Apr 8, 2025

Was there a matching PR/branch for system_tests repo? I couldn't see one

yes, i definitely forgot to push it and didn't do it just now: ISISComputingGroup/system_tests#128

@Tom-Willemsen
Copy link
Member

Sphinx build says ❌ with latest merge - mind having a look?

@rerpha rerpha force-pushed the consolidate_imports branch from 65ec2c6 to d215587 Compare April 10, 2025 13:44
@rerpha rerpha requested a review from Tom-Willemsen April 14, 2025 13:33
Copy link
Member

@Tom-Willemsen Tom-Willemsen left a comment

Choose a reason for hiding this comment

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

In summary, these changes are aligned with best practices in software development to:

Improve maintainability and readability.
Clearly define the module's public API.
Enhance functionality by adding new features.
Organize the codebase into logically separated components.

@Tom-Willemsen Tom-Willemsen merged commit a5ae3ca into main Apr 14, 2025
13 checks passed
@Tom-Willemsen Tom-Willemsen deleted the consolidate_imports branch April 14, 2025 13:34
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Consolidate import name spaces

3 participants