Closed MicahGale closed 8 months ago
added 13 commits
marked this merge request as ready
changed target branch from develop
to issue5ReadCard
changed target branch from issue5ReadCard
to testing
changed target branch from testing
to issue5ReadCard
In GitLab by @tjlaboss on Feb 19, 2022, 09:36
Commented on tests/test_integration.py line 8
One thing I like to do is store file paths as constants in a module.
Good point. Does it matter too much though for a test suite? This won't be shipping with the packages.
In GitLab by @tjlaboss on Feb 19, 2022, 11:10
Commented on tests/test_data_cards.py line 65
ident
is not used. This does not test uppercase (and it breaks when it does).
In GitLab by @tjlaboss on Feb 19, 2022, 11:10
Commented on tests/test_material.py line 11
Suggest making these classes available in mcnpy
namespace. I'll submit a merge request doing the following:
# __init__.py
from .data_cards.thermal_scattering import ThermalScatteringLaw
In GitLab by @tjlaboss on Feb 19, 2022, 11:10
Commented on tests/test_material.py line 19
Rather than initializing Material(input_card, None)
, define
Class Material(input_card, comment: str=None)
In GitLab by @tjlaboss on Feb 19, 2022, 11:10
Commented on tests/testCellProblem.py line 48
This won't work for imp:n,p=1
In GitLab by @tjlaboss on Feb 19, 2022, 11:14
Commented on tests/test_integration.py line 8
It does not matter too much.
In GitLab by @tjlaboss on Feb 19, 2022, 11:14
Commented on tests/test_data_cards.py line 65
See !10 0ef01f8659324e0963b53ec8b0e5dafae5fc8b16.
I wrote these tests a bit fast and dirty as you can tell.
Right. I currently don't like this dictionary approach to parameters and want to change it to something more custom. So I'm not sure how much effort we want to sink into this method right now.
Yes this should be fixed. Though it's actually a Comment
object.
changed this line in version 6 of the diff
added 11 commits
Let's punt this discussion to another MR for now.
resolved all threads
changed this line in version 7 of the diff
changed this line in version 7 of the diff
changed this line in version 7 of the diff
changed this line in version 8 of the diff
added 9 commits
issue5ReadCard
mentioned in commit 82aa26bd8229c1890f7f598dbaf164e9600151a1
Merges fixUnit -> issue5ReadCard
All passes now.