While solving #341 to implement @particle_input in our physics subpackage, we came across a problem where mass_density takes in an optional Particle argument (otherwise defaults to None).
If we're going to use @particle_input on mass_density, we will end up making it mandatory to pass a Particle when calling mass_density AFAIK there is currently no way to make passing a Particle argument optional with @particle_input.
I see two solutions to this.
None as a Particle when using @particle_input. This should be pretty straight forward to implement (in either Particle class or @particle_input decorator to accept None as a Particle). With this we will able to do stuff like:>>> from plasmapy.atomic import particle_input, Particle
>>> @particle_input
... def make_particle(particle: Particle):
... return particle
>>> make_particle(None)
None
typing.Union in function declaration to accept either a Particle or None. It would look something like this:>>> from plasmapy.atomic import particle_input, Particle
>>> from typing import Union
>>> @particle_input
... def make_particle(particle: Union[Particle, None]):
... return particle
>>> make_particle(None)
None
I personally like the looks of this solution but I don't know whether this would be worth the implementation effort.
I made a PR #528 which adds supports for having a default Particle value when using @particle_input. With the combination of one of above solution we should be able to set default Particle for mass_density to None and it would end up looking like this:
def make_particle(particle: Particle = None):
or
def make_particle(particle: Union[Particle, None] = None):
depending on what solution we go with.
That all said, I think mass_density is the only function as of now which takes in a None for Particle. So, there is a possibility we might be better off refactoring it in a way we don't have to implement the above solutions or we could leave mass_density as is and apply @particle_input decorator on other functions.
Optional[Particle]!
That all said, I think mass_density is the only function as of now which takes in a None for Particle.
But the doc says it takes electron as a default argument?
Under mass_density():
Representation of the particle species (e.g.,
'p'for protons,'D+'
for deuterium, or'He-4 +1'for singly ionized helium-4),
which defaults to electrons.
@ionwyn Nice find! That is probably wrong and docstring should be changed to something like "defaults to None".
Wow, I just noticed @particle_input already takes in an optional argument none_shall_pass which accepts None values where Particle is expected. So, this means that code like this should work properly
>>> from plasmapy.atomic import particle_input, Particle
>>> @particle_input(none_shall_pass=True)
... def happy_particle(particle: Particle):
... return particle
>>> happy_particle(None)
None
where as this would raise a TypeError when none_shall_pass is set to False (by default)
>>> from plasmapy.atomic import particle_input, Particle
>>> @particle_input
... def happy_particle(particle: Particle):
... return particle
>>> happy_particle(None)
TypeError: The argument particles to happy_particle must be a string, an integer or a tuple or list of them corresponding to an atomic number, or a Particle object.
I think this implementation works good for the moment and we could stick to it as is. I checked that this doesn't work properly when a function parameter expects a collection of Particles, I could fix this however. Also, maybe we could add it in this error message that "pass none_shall_pass=True to accept None as a Particle" as this wasn't obvious to me.
@ritiek I see. I didn't notice that either. That's a very nice find!
It was either @StanczakDominik or @namurphy that implemented that I think... great name in any case :laughing:
Most helpful comment
It was either @StanczakDominik or @namurphy that implemented that I think... great name in any case :laughing: