The @particle_input decorator and Particle class were recently created in order to take an object-oriented approach to atomic data, and simplify how particles as arguments are passed into functions. The @particle_input decorator documentation and Particle class documentation discuss how to use these.
The @particle_input decorator should be implemented in functions that take a particle as an input, and the resulting instance of the Particle class should be used to get things like a particle's mass and charge. In situations where the particle is required to be, e.g., charged, then the require, any_of, and exclude keywords in @particle_input should be used.
The files to be updated are:
We'll need to think some more about how to handle situations where the mean integer charge is a possible argument. If any problems arise with the decorator or class in parameters_cython.pyx, please raise an issue since these are not tested in Cython. This issue should also wait until after #340 is merged. Working with collisions.py should also wait until after #336 is resolved, so that @particle_input can be used for arguments that accept tuples or lists of particles. The Particle class was created in #239 and the @particle_input decorator was created in #242.
Thanks!
-Nick
This is also related to #309.
@ionwyn Would you like to give this a try? Feel free to ask about anything. :)
@ritiek Sure! Thanks for offering the task and help!
The acceptance criteria is clear to me, but I'll only work on it on Friday or weekend as I have a work project due this sprint :) I'll let you know if I have questions. Cheers!
Thank you, @ionwyn!
Hello, everyone. Sorry I started later than I said; busy at work but it's more relaxed now :)
Here's my first take, I want to verify correctness before moving on to other functions.
ebc1c7ba26db5d05e74b4c3259606fee1b6ceb60
Yes, I know that I have not updated the tests for this, yet. I also want to ask about naming convention, as you can see particle.particle may seem awkward, since the Particle class has class variable named particle.
Again, thanks for giving me the opportunity to contribute to this awesome project!
Looks a good start to me!
I realized this after checking your changes, it would be a good idea to accept default parameters as None with @particle_input. Otherwise we would now be making mass_density mandatory to take in a Particle when it is supposed to be optional (would defaults to None). I'll take a look into what can be done here soon.
I also want to ask about naming convention, as you can see
particle.particlemay seem awkward, since theParticleclass has class variable named particle.
Yep, seems a bit awkward to me as well but we're probably going to keep at it if there is no alternative. That said, we might need to give a better look here if this could be done in a cleaner way.
I think I saw it on #528 . Thanks!
Yep, with the combination of #528 and a way to deal with optional Particle argument defaulting to None we should be able to handle this. I opened an issue on this #530.
Cool, thanks!
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.
I'll just work on other functions in the mean time then!
@ionwyn So it seems like we could set none_shall_pass=True in @particle_input and then mass_density function could optionally accept particles like so particle: Particle = None without resulting in an error as we saw in https://github.com/PlasmaPy/PlasmaPy/issues/530#issuecomment-412517426.
However, in yesterday's meeting we discussed that particle: Optional[Particle] looks nicer than the above solution. So, we would eventually have to implement this functionality to accept Optional[Particle] annotation in functions decorated with @particle_input in a later PR. But for now, let's get this issue done with setting none_shall_pass to True.
@ritiek on it. Sorry I've been mia for a while
@ionwyn Don't worry about it. This issue isn't a high priority, so take your time. :)