-
Notifications
You must be signed in to change notification settings - Fork 18
882 add missingdatamixin #884
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
…est_missing_data_mixin.py
…ine logic in MissingDataMixin
…g data and deadlines
…ngDataMixin tests
|
Small question: The branch this gets merged into doesn't have a PR itself? (or maybe I was blind) @cpaniaguam |
digicosmos86
left a comment
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.
Generally this looks good! I think MissingDataMixin should hold internal properties related to missing data and deadline, so that _set_missing_data_and_deadline does not have to be a static method
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.
I wonder if MissingDataMixin should manage its internal properties instead of just an encapsulation of functions?
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.
I thought of this but it seems like mixins should be stateless. And I tend to agree with this school of thought as it could be difficult to handle __init__ collisions in the MRO. This way we won't have to manage complex super() chains across multiple inherited entities.
This pull request refactors and centralizes the logic for handling missing data and deadline processing in the HSSM codebase. The main changes introduce a new
MissingDataMixinto encapsulate all missing data and deadline-related logic, and update both theHSSMBaseandHSSMclasses to use this mixin. This improves code organization, maintainability, and reusability by avoiding code duplication and clarifying responsibilities.Refactoring and centralization of missing data logic:
MissingDataMixininsrc/hssm/missing_data_mixin.pythat provides methods for handling missing data and deadline logic, including_handle_missing_data_and_deadline,_set_missing_data_and_deadline, and_process_missing_data_and_deadline. This mixin consolidates logic previously scattered inHSSMBaseandDataValidatorMixin.HSSMBaseandHSSMclasses to inherit fromMissingDataMixin, replacing their internal missing data and deadline logic with calls to the new mixin methods. This removes redundant code and ensures consistent handling across classes. [1] [2] [3] [4]Code cleanup and organization:
_handle_missing_data_and_deadlinemethod fromDataValidatorMixinand related logic fromHSSMBase, as this functionality is now provided by the mixin. [1] [2]HSSMBase.__init__by grouping initialization steps into logical regions using comments, and delegating missing data/deadline processing to the mixin. [1] [2] [3]Test updates:
_handle_missing_data_and_deadlinemethod fromtest_data_validator.py, as this logic is now handled by the mixin and tested elsewhere.test_hssmbase.pyas expected to fail in CI to improve test reliability.