-
Notifications
You must be signed in to change notification settings - Fork 306
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
Issue 2673: Rename binding_energy
to remove ambiguity, add electron_binding_energy
#2693
base: main
Are you sure you want to change the base?
Conversation
Thank you for submitting a pull request (PR) to PlasmaPy! ✨ The future of the project depends on contributors like you, so we deeply appreciate it! 🌱 Our contributor guide has information on:
Important PlasmaPy recently switched to an The bottom of this page shows several checks that are run for every PR. Don't worry if something broke! We break stuff all the time. 😺 Click on "Details" to learn why a check didn't pass. Please also feel free to ask for help. We do that all the time as well. 🌸 You can find us in our chat room or weekly community meeting & office hours. Here are some tips:
If this PR is marked as ready for review, someone should stop by to provide a code review and offer suggestions soon. ✅ If you don't get a review within a few days, please feel free to send us a reminder. Please also use SI units within PlasmaPy, except when there is strong justification otherwise or in some examples. We thank you once again! |
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #2693 +/- ##
==========================================
+ Coverage 94.90% 95.24% +0.34%
==========================================
Files 104 104
Lines 9435 9468 +33
Branches 2159 2171 +12
==========================================
+ Hits 8954 9018 +64
+ Misses 299 273 -26
+ Partials 182 177 -5 ☔ View full report in Codecov by Sentry. |
@pheuer - here's a new PR from a fresh branch! |
Thank you!!! Here's a couple comments:
Particle("C +3").electron_binding_energy == Particle("C-12+3").electron_binding_energy
>True Right now the isotope just raises an exception. Again, should work for everything above hydrogen, but return separate values for the hydrogen isotopes since the ionization energies are measurably different there.
Particle("C +3").electron_binding_energy.to(u.eV)
> 882.08 eV But according to the NIST database, that's the ionization energy of C+4, and C+3 should be Otherwise this looks great to me, thanks again for doing this! |
Description
Adds requested feature from #2673.
binding_energy
tonuclear_binding_energy
, but leavesbinding_energy
as a pointer tonuclear_binding_energy
for backwards compatibility with a warning that it is deprecated and will be removed in a future release.electron_binding_energy
, defined as the sum ofionization_energy
for all charge states above the given ionization.Motivation and context
See #2673. With the addition of ionization energy data, calculating
electron_binding_data
becomes fairly easy. However, this causesbinding_energy
to be ambiguous/confusing, and is renamed tonuclear_binding_energy
for clarity and greater specificity.Related issues
Resolves #2673.