-
Notifications
You must be signed in to change notification settings - Fork 15
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
refactor: dormancy calculations #1413
Conversation
Reviewer's Guide by SourceryThis pull request refactors the dormancy calculations in the RAMSTK project, focusing on improving type hints, error handling, and test coverage. The changes primarily affect the Updated class diagram for dormancy calculationsclassDiagram
class DormancyCalculations {
+get_environment_type(env_id: int, is_active: bool) Optional~str~
+get_dormant_hr_multiplier(hw_info: List~Union~int, float~~, env_active: str, env_dormant: str) float
+do_calculate_dormant_hazard_rate(hw_info: List~Union~int, float~~, env_info: List~int~) float
}
DormancyCalculations : -DORMANT_HR_MULTIPLIER
DormancyCalculations : +get_environment_type
DormancyCalculations : +get_dormant_hr_multiplier
DormancyCalculations : +do_calculate_dormant_hazard_rate
DormancyCalculations : +ENVIRONMENTS_ACTIVE
DormancyCalculations : +ENVIRONMENTS_DORMANT
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
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.
Hey @weibullguy - I've reviewed your changes - here's some feedback:
Overall Comments:
- Consider adding a comment in the
get_environment_type
function explaining whyNone
is returned for invalid inputs, to make the intention clear for future maintainers.
Here's what I looked at during the review
- 🟡 General issues: 1 issue found
- 🟢 Security: all looks good
- 🟢 Testing: all looks good
- 🟢 Complexity: all looks good
- 🟢 Documentation: all looks good
Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
Does this PR introduce a breaking change?
Describe the purpose of this pull request.
Added detailed type hints throughout the code to ensure clarity on what each parameter and return value is expected to be.
The get_environment_type function now returns Optional[str], indicating that it may return None if the environment ID is invalid.
Instead of potentially allowing the get_environment_type function to return an invalid environment string, it now explicitly returns None if the env_id is out of bounds. The do_calculate_dormant_hazard_rate function handles this case by returning 0.0 if either environment is invalid.
Clarified hw_info indices in the docstrings to help understand what each index represents.
The code that handles the special case for semiconductors (category 1) was clarified, reducing unnecessary branches and making the logic easier to follow.
Describe how this was implemented.
Ran code through ChatGPT.
Describe any particular area(s) reviewers should focus on.
None
Provide any other pertinent information.
Pull Request Checklist
Code Style
Static Checks
this PR.
Tests
Chores
this PR. These problem areas have been decorated with an ISSUE: # comment.
Summary by Sourcery
Refactor dormancy calculations to improve clarity and robustness by handling invalid environment IDs and simplifying logic for semiconductors. Enhance type hinting and expand test coverage to ensure correct behavior in edge cases.
Enhancements:
Tests: