Rename plasmapy.atomic to plasmapy.particles #742
Conversation
In preparation for renaming `plasmapy.atomic` to `plasmapy.particles`, I went through and changed variables named `particles` to `species` to avoid the namespace conflicts that arose when I first trying the Great Renaming. While I was at it, I also changed `ion_particle` to `ion` whenever it showed up. This changes the API, in preparation for more changes to the API.
PyCharm made this so easy! The only thing I had to change manually was a docstring example that imported plasmapy.atomic. Now to check the docstrings... I'll be thanking Software Carpentry for teaching me how to use grep.
namurphy
added
plasmapy.particles
|
Hello @namurphy! Thanks for updating your pull request.
Comment last updated at 2020-02-04 20:23:31 UTC |
namurphy
force-pushed
the
rename-atomic-to-particle
branch
from
26200c5 to
a9581d1
Compare
Codecov Report
@@ Coverage Diff @@
## master #742 +/- ##
==========================================
+ Coverage 95.06% 96.23% +1.16%
==========================================
Files 56 55 -1
Lines 5047 4967 -80
==========================================
- Hits 4798 4780 -18
+ Misses 249 187 -62
Continue to review full report at Codecov.
|
The only places where "plasmapy.atomic" should show up in the repository are in the change log and release notes. Those references should stay as they are because they refer to past releases.
|
What you could do here is merge this directly into the tester helpers pr first, then squash merge this, then squash merge the latter. I think git will handle it. Also grep is great! Ripgrep is a bit nicer on the interface and faster, but that's personal preference. Glad you're picking these tricks up! |
There was a problem hiding this comment.
I gave these changes a cursory look through - I mean, they're mostly automatic anyway - and they look okay. What I'd like to suggest is keeping plasmapy.atomic around for one release (less than a year this time!) as an alias to particles that also DeprecationWarns. We actually have users for this part (ping the illustrious @wtbarnes ) and it'd be rude not to, I think.
Yes, that is a real issue. I raised an issue in the fiasco repository (wtbarnes/fiasco#52), and offered to submit a PR there to account for this API change after it gets included in a release. There might be one or two more repositories too. There has not yet been an official release of fiasco so the PR could suffice. @wtbarnes, if there's an approach to this that would work particularly well for you, please let us know! Since import particles
import particles as atomic
import formulary
import formulary as physics...though we would want to had DeprecationWarnings in there too. |
🙀 |
|
Admittedly I don't worry about formulary as much because I'm not aware of users for those yet - and if there are any, I'd imagine surface level data analysis to be easier to change than libraries e.g. fiasco. |
|
I sincerely appreciate the effort to accommodate fiasco but I think breaking changes between minor versions pre 1.0 is perfectly acceptable. By depending on a pre-1.0 package like plasmapy, fiasco is accepting the risk that things might break and that is ok! And as you say, fiasco has not even had a formal release yet so changing things up to accommodate upstream issues is not a problem at all. I’m happy to do it. I would recommend just doing whatever makes your life as a developer easiest! Thanks for keeping me in the koop! |
|
Thank you for your flexibility and understanding, @wtbarnes! We are trying to get breaking changes done sooner rather than later. I'll put the question about merging this on the agenda for next week's community meeting. |
|
Or, since @wtbarnes is being gloriously agile in mindset about this (and this is the first time I use the term non-sarcastically), we could simply do the brute force renaming :) |
As part of the changes associated with the forthcoming PLEP 7 (PlasmaPy/PlasmaPy-PLEPs#26), we decided to rename
plasmapy.atomictoplasmapy.particles. One of the primary reasons for this is that this subpackage includes not just atomic data but also data and representations for fundamental particles. Needless to say, this PR is a huge API chance.particlesas a variable in a bunch of different places. I went through and changed these to avoid namespace conflicts.ion_particletoionin a few places incollisions.py,braginskii.py, and related files.plasmapy.atomictoplasmapy.particles. This amazingly almost everything that was necessary in the code, and I only had to fix one test failure.docsdirectory more carefully too, and changed a few section titles. Hopefully the docs build okay...grepto find any lingering places where atomic was used. I'm hugely grateful for Software Carpentry teaching me this command.The rebase associated with #728 might be a pain since I had to update a bunch of files in
plasmapy.atomicin small ways while rearranging some formatting things inplasmapy.utils. In order to (possibly? hopefully?) make rebasing easier, I'd suggest not doing a squash merge on this one so that git can decipher all of the little changes separately. Wish me luck! Or perhaps more specifically, wish me good luck....