Implement __eq__ for AbstractParticle

[vrajashkr]

• I have added a changelog entry for this pull request.

• If adding new functionality, I have added tests and
  docstrings.

• I have fixed any newly failing tests.

Closes #806
May close #848

Implements __eq__ into AbstractParticle class, allowing comparisons of CustomParticle and DimensionlessParticle using == operator.

[codecov]

Codecov Report

Merging #873 into master will decrease coverage by 0.02%.
The diff coverage is 100.00%.

@@            Coverage Diff             @@
##           master     #873      +/-   ##
==========================================
- Coverage   96.13%   96.11%   -0.03%     
==========================================
  Files          60       60              
  Lines        5309     5277      -32     
==========================================
- Hits         5104     5072      -32     
  Misses        205      205              

Impacted Files	Coverage Δ
plasmapy/particles/particle_class.py	96.61% <100.00%> (+0.03%)	⬆️
plasmapy/plasma/plasma_base.py	67.74% <0.00%> (-4.49%)	⬇️
plasmapy/simulation/abstractions.py	75.00% <0.00%> (-2.78%)	⬇️
plasmapy/plasma/sources/openpmd_hdf5.py	78.40% <0.00%> (-1.17%)	⬇️
plasmapy/simulation/particletracker.py	98.27% <0.00%> (-0.03%)	⬇️
plasmapy/particles/ionization_state.py	92.93% <0.00%> (-0.03%)	⬇️
plasmapy/utils/pytest_helpers/pytest_helpers.py	93.38% <0.00%> (-0.03%)	⬇️
plasmapy/formulary/braginskii.py	99.72% <0.00%> (-0.01%)	⬇️
plasmapy/particles/serialization.py	100.00% <0.00%> (ø)
... and 6 more

---

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 0997e67...da7364a. Read the comment docs.

[vrajashkr]

This commit is a proof of concept and experiment for the mentioned issues.

A few queries I had in mind:

• I had a look at __eq__ implemented for Particle and noticed that there is support for comparing against strings. Should that be supported in this case as well?
• Would it be possible to remove the __eq__ implementation for Particle in favour of the one implemented for AbstractParticle (involves generalizing the __eq__ implementation in 'AbstractParticleto fit the needs ofParticle`)?

Thank you!

[StanczakDominik]

I had a look at eq implemented for Particle and noticed that there is support for comparing against strings. Should that be supported in this case as well?

Well... for a Particle("e-") you can definitely state that it represents the same particle as the string "e-". For CustomParticles, you're getting into float comparison... I'd say nah.

Would it be possible to remove the eq implementation for Particle in favour of the one implemented for AbstractParticle (involves generalizing the eq implementation in 'AbstractParticleto fit the needs ofParticle`)?

Ideally, yeah! That seems like a good idea.

[vrajashkr]

Well... for a Particle("e-") you can definitely state that it represents the same particle as the string "e-". For CustomParticles, you're getting into float comparison... I'd say nah.

I agree. Best to avoid checking for equivalence between particles and string when the particle has a charge and a mass.

Would it be possible to remove the eq implementation for Particle in favour of the one implemented for AbstractParticle (involves generalizing the eq implementation in 'AbstractParticleto fit the needs ofParticle`)?

Ideally, yeah! That seems like a good idea.

This might be tricky to generalise though. For CustomParticle and DimensionlessParticle, we can generalise because we can compare masses and charges, however, we can't do the same for Particle since a mass and charge comparison would not be sufficient to call them equivalent. Any suggestions on how we could handle this particular case are welcome !

Thank you!

[namurphy]

I'm wondering a bit more about how we should treat __eq__ specifically for a DimensionlessParticle. To nondimensionalize a particle for a real or a theoretical application (ignoring implementation in PlasmaPy completely), we divide the mass by a characteristic mass, and divide the charge by a characteristic charge. In those applications, the true value of the mass and charge depends on the normalization, which is not contained within the DimensionlessParticle class. Strictly speaking, two dimensionless particles would only be equal if the normalized mass and charge are the same.

With all that said, I still think it's reasonable to define DimensionlessParticle as having the same dimensionless charge and mass, since that's the behavior that most people would expect. We'd just want to make sure to document what it's doing.

[vrajashkr]

To nondimensionalize a particle for a real or a theoretical application (ignoring implementation in PlasmaPy completely), we divide the mass by a characteristic mass, and divide the charge by a characteristic charge. In those applications, the true value of the mass and charge depends on the normalization, which is not contained within the DimensionlessParticle class. Strictly speaking, two dimensionless particles would only be equal if the normalized mass and charge are the same.

This is interesting and raises a question. At the moment, the 3 classes inheriting from AbstractParticle have 3 different implementations of __eq__. Could another approach be to make __eq__ an abstract method in AbstractParticle and implement inside each class separately? The benefit of this approach is that it keeps the comparison consistent with standard comparisons for the particles that are currently there as well as for any future particles that might be added.

With all that said, I still think it's reasonable to define DimensionlessParticle as having the same dimensionless charge and mass, since that's the behavior that most people would expect. We'd just want to make sure to document what it's doing.

This works too, however, may require redesigning and potential deprecation of some features from __eq__ for Particle. For example, equality with strings may need to be deprecated in order to implement the method effectively in AbstractParticle.

[rocco8773]

My opinion, defining __eq__ and __ne__ for AbstractParticle and then letting the sub-classes override or super() __eq__, if necessary, is the way to go. This approach is better at standardizing the comparison and automatically makes the comparison available for all sub-classes. __eq__ for AbstractParticle should focus only on the methods defined within the AbstractParticle scope, i.e.

1. check for class instance isinstance(other, self.__class__)
2. check for equal masses self.mass == other.mass
3. check for equal charges self.charge == other.charge

With this approach, I don't think the flavor of mass or charge will be an issue because if they're a float, int, astropy Quantity, etc. they all have functionality for comparisons.

For the Particle class you would then super() this and add additional comparisons, e.g.

class Particle(AbstractParticle):
    def __eq__(self, other):
        super().__eq__(other)

        # add additional comparisons
        # basically a deep comparison of the self._attributes dictionary

---

I believe this is the approach you're already doing. So, from my perspective, there's two points to address...

1. When comparing mass and charge, do we want to use the u.isclose or ==? I'm not sure which would be better here. Not sure if we should have the "is close" comparison or an exact comparison. ... @namurphy @StanczakDominik Thoughts? I lean towards == and rely on the __eq__ method defined for floats, ints, Quantities, etc.
2. We need to review/update the __eq__ method of the sub-classes.