-
Notifications
You must be signed in to change notification settings - Fork 12
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
Develop combine #659
Develop combine #659
Conversation
…adding_3d_mesh_tally
…adding_3d_mesh_tally
…adding_3d_mesh_tally
…adding_3d_mesh_tally
…adding_3d_mesh_tally
I think that is about as many tests that can be added without testing the importErrors |
John has a few more tests in progress #680 which could help raise the coverage a bit more |
Super! I think the coverage is fine tbh. This can surely be merged in if needed. The question was more on "should we have these try/except imports or not" but this can be answered later :-) |
@billingsley-john that last commit replaced nested tests with discrete unit tests for the test_NeutronicModel |
Thanks for the tests refactoring John, I think this is ready to merge. I will have a go at the import testing at some point and see if it can be done in a simple manner |
I just added a few more suggestions for the pullrequest.com review. Making steady progress getting through them 👍 |
The pullrequest.com suggestions included changing a few ValueErrors to TypeErrors. I forgot to follow this up in the tests so that is why the tests failed. I think they are all fixed now :-) |
@shimwell any idea why the GA CI fails ? |
I think it was due to a small change in the OpenMC DAGMC interface that is fixed now openmc-dev/openmc#1747 I am now rerunning the CI |
Tests are passing on both github actions and circle ci. Happy to merge this one in. |
Proposed changes
This brings in recent changes that have been combined
3d mesh tallies
automatic movement of the trelis script for neutronics simulations
type hinting on lots of classes and functions
Types of changes
Checklist