Bringing SaltProc repository up to date#47
Conversation
|
Hello @andrewryh! Thanks for updating this PR. We checked the lines you've touched for PEP 8 issues, and found: There are currently no PEP 8 issues detected in this Pull Request. Cheers! 🍻 Comment last updated at 2020-08-14 18:47:53 UTC |
mehmeturkmen
left a comment
There was a problem hiding this comment.
@andrewryh As far as I see, there is only one question mark in this PR. In "test_app.py", did you forget to add
procs['fuel']['entrainment_separator'].efficiency['Xe'] == 0.8
to line 44.
| assert procs['fuel']['sparger'].efficiency['Xe'] == 0.6 | ||
| assert procs['fuel']['entrainment_separator'].efficiency['H'] == 0.15 | ||
| assert procs['fuel']['entrainment_separator'].efficiency['Kr'] == 0.87 | ||
| assert procs['ctrlPois']['removal_tb_dy'].volume == 11.0 |
There was a problem hiding this comment.
Did you forget to add "assert procs['fuel']['entrainment_separator'].efficiency['Xe'] == 0.8"? You removed "'Xe': 0.8}" from line 44?
There was a problem hiding this comment.
Right now, in tests, Xe removal efficiency for the entrainment_separator is defined as equation (see saltproc/tests/processes.json).
So, it should be
assert procs['fuel']['entrainment_separator'].efficiency['Xe'] == (string, equation)
@mehmeturkmen
I would leave it as is, if you don't mind.
|
@mehmeturkmen Any other concerns? |
mehmeturkmen
left a comment
There was a problem hiding this comment.
So far, I have no concern. It seems your tests are working well. I am gonna rebase it.
This PR contains all changes I made in SaltProc during the last 5 months.