-
-
Notifications
You must be signed in to change notification settings - Fork 404
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
Add catch for missing elements in SDEC plot #1517
Add catch for missing elements in SDEC plot #1517
Conversation
The elements coloured in the plot are determined via the absorption and emission. It's possible however, that these elements are not the same, e.g. if you specify a limited wavelength range. This could give an error if you try to plot an element not in the emission dataframe, for example. This adds a try statement to catch this case
Codecov Report
@@ Coverage Diff @@
## master #1517 +/- ##
==========================================
- Coverage 67.70% 67.57% -0.14%
==========================================
Files 68 68
Lines 6091 6103 +12
==========================================
Hits 4124 4124
- Misses 1967 1979 +12
Continue to review full report at Codecov.
|
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 ran some checks and you were right about the original code throwing an error in the narrow wavelength regime if there were different emission and absorption elements, so good spot! Ran some tests with the proposed fix, and it works perfectly. The output message on the exception is a particularly nice feature. Good job!
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.
Looks good 👍
* Add catch for missing elements The elements coloured in the plot are determined via the absorption and emission. It's possible however, that these elements are not the same, e.g. if you specify a limited wavelength range. This could give an error if you try to plot an element not in the emission dataframe, for example. This adds a try statement to catch this case * Fixed typo * Black formatting * Fixed typo
This fixes #1516.
The elements coloured in the plot are determined via the absorption and emission. It's possible however, that these elements are not the same, e.g. if you specify a limited wavelength range. For example, you could have a small wavelength region where Fe is absorbed, but not emitted. This could give an error if you try to plot an element not in the emission dataframe, for example. This adds a try statement to catch this case.
How Has This Been Tested?
I've tried artificially adding new elements to the elements list that are not included in the model. This produces the following:
Types of changes
Checklist: