Plasmapy: Accept `NoneType` as a Particle in `@particle_input`

Created on 4 Aug 2018  路  6Comments  路  Source: PlasmaPy/PlasmaPy

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).

https://github.com/PlasmaPy/PlasmaPy/blob/9bcbe0bfd30cc434ccf8389ca84163d342dd0c93/plasmapy/physics/parameters.py#L67-L68

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.


  1. Accept 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

  1. We could use 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.

Particles

Most helpful comment

It was either @StanczakDominik or @namurphy that implemented that I think... great name in any case :laughing:

All 6 comments

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:

Was this page helpful?
0 / 5 - 0 ratings