-
-
Notifications
You must be signed in to change notification settings - Fork 435
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
Replacing PyNE with radioactivedecay for radionuclide decay calculations #1813
Conversation
Before a pull request is accepted, it must meet the following criteria:
|
1 similar comment
Before a pull request is accepted, it must meet the following criteria:
|
Need to format to |
@lemointm the tests are failing - can you make sure it reruns. |
Just pushed a commit for formatting, I will check the other tests next |
pyne also needs to be replaced in the tests ImportError while loading conftest '/home/vsts/work/1/s/tardis/tardis/conftest.py'. |
There are several references to |
There's an ongoing test failure where |
@andrewfullard I added a util to tardis to perform those checks that I had rewritten in a few different places. The code should be a little cleaner now. |
I spoke too soon, I just discovered that |
There also was an error where |
Now there should be only the Ni-58 test error (for now). I will give an update once the fixes are made to |
After the newest release (0.4.12) to |
Codecov Report
@@ Coverage Diff @@
## master #1813 +/- ##
==========================================
+ Coverage 58.74% 58.80% +0.05%
==========================================
Files 70 70
Lines 7847 7853 +6
==========================================
+ Hits 4610 4618 +8
+ Misses 3237 3235 -2
📣 Codecov can now indicate which changes are the most critical in Pull Requests. Learn more |
Can you update the astropy version to >=4.1 to remove the distutils warnings? |
I also added radioactivedecay>=0.4.12 to ensure that the latest version with Ni-58 is installed. |
There still seems to be an issue with astroPy, now with importing |
Reminder to revert the astropy version change in this PR so we can merge it |
Also, please try to build the documentation before merging! |
Black check is failing. |
#1940 is the reason |
Thanks, I think we should update |
I just reverted the astropy version, and ran the docs on my machine - everything looks good! |
@@ -14,7 +14,7 @@ dependencies: | |||
- astropy=3 | |||
- numba=0.53 | |||
- numexpr | |||
- pyne=0.7=nomoab_openmc* # Issue #1537 | |||
- radioactivedecay>=0.4.12 |
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.
Is a good idea to pin the bugfix number? Maybe just minor version it's ok, what do you think @lemointm ?
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.
I talked to Alex about this, we think that in the future the functionality TARDIS needs won't get deprecated
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.
but that shouldn't affect the bugfix number, right?
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.
I see what you're saying, sorry I misunderstood. The "bugfix" added some functionality, so it needs to be pinned... which is not the standard but that's how it ended up I suppose
This PR is designed to replace the functionality of PyNE in TARDIS with the python package
radioactivedecay
.Description
TARDIS uses PyNE to perform decay caluclations, and to handle nuclide name parsing. The main change will be with handling nuclide decays in
tardis/io/decay.py
withradioactivedecay
, and to replace the usage of PyNE'snucname
function with theradioactivedecay.Nuclide
class' name parsing functionality throughout TARDIS. Nuclide decays will now use theradioactivedecay.Inventory
objects and thedecay()
method.Motivation and context
This PR is meant to solve problems outlined in issue #1654.
How has this been tested?
Examples
Type of change
Checklist