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

Make ParticleList compatible with Quantity objects with physical type of mass or electrical charge #1872

Closed
namurphy opened this issue Jan 5, 2023 · 3 comments
Labels
feature request Issues requesting a new feature or enhancement plasmapy.particles Related to the plasmapy.particles subpackage

Comments

@namurphy
Copy link
Member

namurphy commented Jan 5, 2023

Feature description

Right now if we try to create a ParticleList using a Quantity array, we get an InvalidParticleError:

>>> import astropy.units as u
>>> ParticleList([5, 6] * u.kg)
plasmapy.particles.exceptions.InvalidParticleError: The object 5.0 kg supplied to ParticleList is not a particle-like object.

What I'd like to happen is for a CustomParticle to be created based on each element of the Quantity array, i.e.:

>>> ParticleList([5, 6] * u.kg)
ParticleList(['CustomParticle(mass=5.0 kg, charge=nan C)', 'CustomParticle(mass=6.0 kg, charge=nan C)'])

Motivation

As part of the refactoring of particle_input in #1057, we're making it so that a Quantity of physical type mass or electrical charge will be used to construct a CustomParticle instance. I discovered in #1871 that ParticleList works for Quantity non-arrays but not for Quantity arrays. The code downstream of ParticleList will be cleaner and have fewer special cases to handle if ParticleList can handle Quantity iterables of these physical types.

Implementation strategy

We'll need to make these changes to not only ParticleList.__init__, but also the append, extend, and insert methods.

There's some repeated code in these methods, so it might be worth creating a private function that does the particleification or particlizing.

Additional context

Implementing this would probably be cleaner by allowing mass and charge as positional arguments in either order in CustomParticle, kind of like what I attempted in #1364, but maybe having a signature like CustomParticle(*quantities, mass: Optional[u.kg] = None, charge=Optional[u.C] = None, symbol: Optional[str]=None).

@StanczakDominik
Copy link
Member

Or - and bear with me here, because this would of course be a bigger rework - we could go the other way around. Instead of going with a "array of structures" layout, we could go with a "structure of arrays" one. Just have ParticleList refer to underlying astropy quantities for its properties like charge, mass - but just implement a custom __getitem__ that, if you do particle_list[42], pulls the properties from those underlying arrays and generates a Particle or CustomParticle or whatever. That might even simplify it a fair bit!

@namurphy
Copy link
Member Author

namurphy commented Jan 6, 2023

Or - and bear with me here, because this would of course be a bigger rework - we could go the other way around. Instead of going with a "array of structures" layout, we could go with a "structure of arrays" one. Just have ParticleList refer to underlying astropy quantities for its properties like charge, mass - but just implement a custom __getitem__ that, if you do particle_list[42], pulls the properties from those underlying arrays and generates a Particle or CustomParticle or whatever. That might even simplify it a fair bit!

Interesting! If a ParticleList were to contain only CustomParticle objects, that approach would indeed make it cleaner...kind of like making a ParticleList into a generator. What I think would make this difficult to implement is that an iterable used to create a ParticleList can be quite heterogeneous. The inputs could include Particle, CustomParticle, string, integers representing atomic numbers, or (after this issue is addressed) Quantity instances for either charge or mass. In particular for string inputs, creating a Particle is the quickest way of making sure that each item from the iterable is valid. All of this has made it more practical to create the Particle and CustomParticle instances first rather than creating them as needed.

@github-actions github-actions bot added the Stale Dormant issues & PRs which will be automatically closed if the label is not removed. label Mar 8, 2023
@namurphy namurphy removed the Stale Dormant issues & PRs which will be automatically closed if the label is not removed. label May 12, 2023
@namurphy namurphy added the plasmapy.particles Related to the plasmapy.particles subpackage label May 23, 2023
@namurphy
Copy link
Member Author

Closed by #1874.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature request Issues requesting a new feature or enhancement plasmapy.particles Related to the plasmapy.particles subpackage
Projects
None yet
2 participants