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

Update @particle_input to handle typing that utilzes Union[], List[], etc. #912

Open
rocco8773 opened this issue Oct 8, 2020 · 10 comments
Assignees
Labels
Plasma Lv0 | Novice Issues that do not require knowledge of physics plasmapy.particles Related to the plasmapy.particles subpackage Python Lv3 | Proficient Issues that require proficiency in Python
Milestone

Comments

@rocco8773
Copy link
Member

@particle_input ques off of arguments annotated with Particle to properly condition that argument into a Particle() object. However, that argument is allowed to be a string (e.g. "He+") or a Particle() object and, thus, should really be annotated with Union[str, Particle]. Without this annotation, IDEs will generally throw warnings if you trying to use "He+" as an argument.

I think @particle_input can accomplish this by doing something like...

def find_particle(func):
     def find_in_depth(args):
         for arg in args:
             if hasattr(arg, "__args__"):
                 result = find_in_depth(arg.__args__)
             else:
                 result = Particle == arg
             
             if result:
                 break
 
         return result
 
     annos = func.__annotations__
     found = {}
 
     for name, anno in annos.items():
         if hasattr(anno, "__args__"):
             results = find_in_depth(anno.__args__)
         else:
             results = Particle == anno

         found[name] = results

     return found

Then the following would result in...

>>> def foo(ion: Union[str, Particle], par: Particle, p2: List[Particle], arg: [u.cm, u.kg]):
...     pass
>>> find_particle(foo)
{'ion': True, 'par': True, 'p2': True, 'arg': False}
@rocco8773 rocco8773 added the plasmapy.particles Related to the plasmapy.particles subpackage label Oct 8, 2020
@namurphy
Copy link
Member

namurphy commented Oct 8, 2020

Hm...I wonder if it would be helpful to define particle_like as Union[Particle, str, numbers.Integral], and include that in the same file as @particle_input so that it can be imported in the rest of the package and used for these annotations. That'd probably be cleaner than having Union[..., ..., ...] when reading code and still be understandable.

There's also a new feature in Python 3.9 (which was just released!) for type hinting generics which might be cleaner eventually (e.g., list[str] instead of List[str]). It's too early to use this for now, though.

@namurphy
Copy link
Member

namurphy commented Oct 8, 2020

Oh...and Particle also accepts integers...which I just realized means that we can do this:

>>> Particle(True)
Particle("H")

Huh.

@rocco8773
Copy link
Member Author

What's False then? Weird!

@namurphy
Copy link
Member

namurphy commented Oct 8, 2020

Particle(False) raises an InvalidParticleError, instead of the antiparticle of Particle(True). Don't know if we need to have a check for this, since Python itself allows operations like True + 1 which gives 2, and NumPy lets us do numpy.isclose(True, False, True - False) which gives True.

@rocco8773
Copy link
Member Author

rocco8773 commented Oct 11, 2020

I'm currently working on this and hoping to open a PR soon on it. Here are my thoughts on the functionality.

  1. We create an typing alias that looks like...

    ParticleLike = typing.Union[str, numbers.Integer, AbstractParticle]

    Note that typing.Optional[ParticleLike] == typing.Union[str, numbers.Integer. AbstractParticle, type(None)].

  2. @particle_input would then work with typing that look like...

    ParticleLike
    Particle  # any subclass of AbstractParticle
    typing.Optional[ParticleLike]
    typing.Optional[Particle]
    List[Particle]
    List[ParticleLike]
    list(ParticleLike)  # the decorator basically remaps list(), [] -> typing.List[]
    Tuple[ParticleLike, Particle, CustomParticle]
    Tuple[ParticleLike, ...]
    tuple(ParticleLike, ParticleLike, Particle)  # the decorator basically remaps tuple(), () -> typing.Tuple[]
  3. @particle_input would not work with...

    Dict[str, ParticleLike]
    typing.Union[float, ParticleLike]
    typing.Tuple[typing.Any, ParticleLike]
    typing.List[type.Union[ParticleLike, typing.List[ParticleLike]]]
    typing.Mapping[str, ParticleLike]

    In the future we can we can allow for more complex typing variations, but to start I think we should stick to simple ones like ParticleLike, List[ParticleLike], Tuple[ParticleLike], and Optional[ParticleLike].

  4. The decorator would raise an Exception under the following conditions...

    1. If ParticleLike is found at a depth of > 1 (annotations are recursively search for ParticleLike).
    2. If a tuple argument does not match the length of the tuple annotation.
    3. If ParticleLike is Union-ed with non-ParticleLike types
  5. If an argument is not an instance of one of the AbstractParticle sub-classes, then Particle() will be used to convert it to a particle. This allows CustomParticle and DimensionlessParticle to pass but non-particle objects will only be converted with Particle().

  6. I'm not a big fan of the decorator pulling Z and mass_numb from from the particle object. This feels beyond the scope of the decorator, but I will leave it in place for now. I have an idea of how to replace this, which I've outlined in issue Decorator @from_particle #918.

@namurphy
Copy link
Member

This looks great, and very thorough! This covers a lot of use cases that I hadn't thought about.

I was thinking that the docstring for this would be a good place to discuss what valid representations of particles are. If you don't mind, I'd like to help out with at least the docstring for this since this is something I've been meaning to put in the documentation anyway.

@rocco8773
Copy link
Member Author

Sure, I've made a bunch of progress in getting the above implement and should be able to open a PR sometime next week. We can go through the docstrings, and more, at that point.

@namurphy
Copy link
Member

Sounds good! I also commented on this in #796. In particular, I tentatively created a one-liner for particle_like and started using it, so we could expand off of that.

@rocco8773
Copy link
Member Author

I'll point out, one of the benefits of having a particle_like typing alias is that we will actually have a documentation page that will always be linked back to when we document with `particle_like`.

@namurphy
Copy link
Member

EXACTLY!

@namurphy namurphy mentioned this issue Oct 13, 2020
7 tasks
@StanczakDominik StanczakDominik added this to the 0.6.0 milestone Feb 23, 2021
@StanczakDominik StanczakDominik modified the milestones: 0.6.0, 0.7.0 Mar 12, 2021
@rocco8773 rocco8773 modified the milestones: 0.7.0, 0.8.0 Jul 20, 2021
@rocco8773 rocco8773 modified the milestones: 0.8.0, 0.9.0 Jun 28, 2022
@namurphy namurphy assigned namurphy and unassigned rocco8773 Sep 9, 2022
@namurphy namurphy modified the milestones: 0.9.0, 0.10.0 Nov 8, 2022
@namurphy namurphy modified the milestones: 2023.1.0, 2023.5.0 Jan 9, 2023
@namurphy namurphy added Python Lv3 | Proficient Issues that require proficiency in Python Plasma Lv0 | Novice Issues that do not require knowledge of physics and removed Project Lv4 | Proficient labels Mar 9, 2023
@namurphy namurphy modified the milestones: 2023.5.0, 2023.9.0 May 12, 2023
@namurphy namurphy modified the milestones: 2023.10.0, v2024.1.0 Oct 22, 2023
@namurphy namurphy modified the milestones: v2024.2.0, Future Feb 6, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Plasma Lv0 | Novice Issues that do not require knowledge of physics plasmapy.particles Related to the plasmapy.particles subpackage Python Lv3 | Proficient Issues that require proficiency in Python
Projects
None yet
Development

No branches or pull requests

3 participants