PlasmaPy / PlasmaPy

An open source Python package for plasma research and education
http://docs.plasmapy.org
BSD 3-Clause "New" or "Revised" License
562 stars 336 forks source link

Decide how `@particle_input` treats parameters named `ion` when provided with neutral atoms #2049

Open namurphy opened 1 year ago

namurphy commented 1 year ago

Suppose we have a function decorated with @particle_input which has a parameter that is named ion which is annotated with ParticleLike.

from plasmapy.particles import particle_input, ParticleLike

@particle_input
def f(ion: ParticleLike):
    return ion

At present, such a function will accept a neutral atom without raising an exception:

>>> f("He-4 0+")
Particle("He-4 0+")

However, strictly speaking, 4He0+ is not an ion, so the naming is imperfect. The name of ion stuck because we haven't figured out a better naming scheme.

In some cases, we use a more general parameter name, like particle, especially for functions that allow CustomParticle objects.

A recent alternative which has been implemented in @particle_input has been ionic_level ( #2034). ChatGPT suggested "atomic species", which appears to be used more often than "ionic level" to refer to ions and neutral atoms. More broadly, we might call it "atom"...but that's a little less explicit. Sometimes we might want CustomParticle objects representing a mean ion to be passed through, though, which doesn't seem to fit within "atomic species".

Anyways, I don't know what we should do, but we should figure this out! We do want to be cautious changing this, because we currently use ion as a parameter name for a bunch of functions, so we might have to break backwards compatibility (or build a workaround and a deprecation warning in @particle_input).

This came up most recently in a discussion in #2034.

pheuer commented 1 year ago

@namurphy I think we should come up with some alternatives and then put together pro/con lists ;) Here's one possible solution:

namurphy commented 1 year ago

Thank you for the reply, and +1 on doing tradeoff analyses! Here are a couple of the tradeoffs that I can think of. Are there any I'm missing?

Change all the special keywords to particle.

Benefits

Drawbacks

Considerations

pheuer commented 1 year ago

Is there a drawback to making ion, atom, etc. all synonyms for particle, so the name in the docs can be ion but particle_input will process it exactly the same as if it was particle, meaning that any explicit requirements (e.g. your example for the Alfven speed) would have to be specified in the particle_input decorator arguments?

namurphy commented 1 year ago

Is there a drawback to making ion, atom, etc. all synonyms for particle, so the name in the docs can be ion but particle_input will process it exactly the same as if it was particle, meaning that any explicit requirements (e.g. your example for the Alfven speed) would have to be specified in the particle_input decorator arguments?

I'd say that the biggest drawback is probably that it would take ∼2 days of work to make this change since there's a lot of code that makes use of the special handling for parameter names of ion, element, or isotope. The special handling of these parameter names also allows us to raise specific exceptions for InvalidIonError, InvalidElementError, and InvalidIsotopeError and have more targeted error messages.

For this particular issue, I'll admit being a bit cautious about scope creep! 🙃 I'm hoping to focus on the narrow case where the parameter is named ion and it gets provided with an argument that is neutrally charged.

namurphy commented 1 year ago

It looks like I also forget to point out the code that does the checks for when the parameter is named ion, element, or isotope. Addressing this issue would have been a lot harder before the great refactoring of #1057!