-
Notifications
You must be signed in to change notification settings - Fork 484
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
Allowing CylindricalMesh and SphericalMesh to be fully made via constructor+ type hints #2619
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.
Thanks for this PR @shimwell!
Co-authored-by: Paul Romano <[email protected]>
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.
In addition to line comments below, it looks like more changes were now made in other files (not relevant to original purpose of PR) -- I'll review those but I'd appreciate if we can limit the scope of the PR in the future
Co-authored-by: Paul Romano <[email protected]>
Co-authored-by: Paul Romano <[email protected]>
I did have to tinker with a few extra test files that were calling openmc.CylindricalMesh() and openmc.SphericalMesh() without arguments (which is no longer not possible as we now have mandatory args) but looks like the CI is now passing 🎉 |
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.
Sorry for the delay on this -- thanks for the updates @shimwell!
Many thanks Paul, sorry it was not the most tidy PR. |
…ructor+ type hints (openmc-dev#2619) Co-authored-by: Paul Romano <[email protected]>
Description
I've been crawling through the code looking for classes that are not fully setable from their args and I found CylindricalMesh and SphericalMesh were just missing the origin arg.
So I've added that argument to their __ init __
While browsing the code I couldn't resist adding a few type hints
Fixes # (issue)
Checklist
- [ ] I have run clang-format on any C++ source files (if applicable)