Skip to content
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

@particle_input does not apply categorization criteria when creating a ParticleList #2048

Closed
namurphy opened this issue Mar 31, 2023 · 2 comments · Fixed by #2594
Closed
Labels
bug Issues describing unexpected behavior or defects. Remember: a bug is a sign of a missing test! plasmapy.particles Related to the plasmapy.particles subpackage priority: high Issues & PRs with significant urgency and importance that should be addressed soon

Comments

@namurphy
Copy link
Member

Bug description

The require, exclude, and any_of parameters of @particle_input do not get enforced when the decorated function gets passed a ParticleList.

Expected outcome

When we use the require, any_of, and exclude parameters with @particle_input, we should make them work with ParticleLike.

@particle_input(require="ion", exclude={"isotope"})
def f(x: ParticleLike):
    return x

Minimal complete verifiable example

>>> from plasmapy.particles import ParticleLike, particle_input
>>> 
>>> @particle_input(require="ion")
... def f(particle: ParticleLike):
...     return particle
...
>>> f(["e-", "e+"])  # should raise an error because these are not ions
ParticleList(['e-', 'e+'])

Package versions

No response

Additional context

When the decorated parameter is named ion (for example), @particle_input does indeed check that the list contains only ions.

>>> @particle_input
... def g(ion: ParticleLike):
...     return ion
...
>>> g(["e-", "e+"])
plasmapy.particles.exceptions.InvalidIonError: The argument ion = ParticleList(['e-', 'e+']) to g does not correspond to a valid ion.

However, there's a little singular vs. plural weirdness in the error message that we could fix.

@namurphy namurphy added the bug Issues describing unexpected behavior or defects. Remember: a bug is a sign of a missing test! label Mar 31, 2023
@namurphy
Copy link
Member Author

Some more thoughts:

  • We could potentially treat parameter names like ions similar to how ion is already treated.
  • A related possibility would be to have any_of, exclude, and require parameters for ParticleList, so that we could do things like:
    >>> ParticleList(["e-", "e+"], require="ion")
    InvalidIonError: ...
    The difficulty would be about whether or not we should have methods like ParticleList.append, etc. also enforce the categorization criteria. This capability would add extra complication to code within ParticleList, so I'm hesitant to do this unless there end up being a couple of use cases that can't be handled by @particle_input.
  • This came up in the same discussion about Collisional analysis #1986 that led to Enable a way to enforce the length of a ParticleList upon creation #2045.

@namurphy namurphy added plasmapy.particles Related to the plasmapy.particles subpackage priority: high Issues & PRs with significant urgency and importance that should be addressed soon labels May 23, 2023
@namurphy
Copy link
Member Author

namurphy commented Mar 21, 2024

Looking back at this, I should note that this does work as expected with:

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

@particle_input
def get_isotope(isotope: ParticleLike):
    return isotope

In the above examples, @particle_input infers from the parameter name what the categorization criteria should be, and raises exceptions as expected.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Issues describing unexpected behavior or defects. Remember: a bug is a sign of a missing test! plasmapy.particles Related to the plasmapy.particles subpackage priority: high Issues & PRs with significant urgency and importance that should be addressed soon
Projects
None yet
Development

Successfully merging a pull request may close this issue.

1 participant