-
Notifications
You must be signed in to change notification settings - Fork 24
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
Idea: move particle search functionality to a new ParticleDB class #277
Comments
Sounds good. Why not make find() and findall() functions on the module-level? Why attach them to a class? I think the fundamental question is whether you want to expose the underlying database to the user or keep it an implementation detail. If you want to expose the database and make it easy for users to change the data source of the database, then it would be better to expose a ParticleTable class or ParticleDB. I am not sure whether this is a good idea. I like the simplicity of particle and I trust it to use the most recent world average data when I import the most recent version. I don't want to use outdated masses and life-times and I cannot imagine people will. So I think it is a good choice to not expose the DB to the user as it is now. |
The ability to add particles, modify particles, or load a new table is a key component to Particle's original design, and is used in several applications, like DecayLanguage, and in at least one analysis. There are a variety of reasons you may want to load specific older data, or modified data, or add particles.
Because Particle was explicitly designed to be simple to use in a REPL. You can access all the functionality with just a If you are not in a REPL,
I like those names better than from particle import ParticleDB
pdb = ParticleDB() # default particle table
p = pdb.find(...) However, this requires a user to learn about Particle databases, to keep their own state ( The current design is a balance between the two uses - you have a simple way to load particles, and an "advanced" way to modify, replace, or extend the particle table. For a proposal to move forward, I would recommend the following: Add a ParticleDB class as seen above, and refactor the current This of course would need to happen after thought is put into what it means to compare two particles that were loaded from different ParticleDB's. |
I'm on the fence here and will need to think carefully, not Friday at the end of the busy week ;-). In any case my really general comment is that we should not forget that so far we hardly ever had a complaint on the API. I would conclude from there that it's pretty good. In other terms, whatever we do should not totally break the way we have people working with the package, as that's likely not so welcome. On general grounds I always liked very much the fact that the user does not have to care with the DB/files, which are behind the scenes. We provide the latest info (and the user can go back in history if they so want) with a powerful API, and that's paramount. Now, if we want to have a, separate, way to "play" with the DB ... OK, more when I get to think about the future of Particle properly :-). Have a good weekend. |
Ok, that's an explanation for the choice, but this is designing for the developer and not designing for the user. Users have no need to derive from Particle. |
The idea has been around in our heads for a while. It came out again in discussions in #263. Let's have this as a possible API for version 1.0.
For reference: indeed the present
Particle
class is both the class for particle properties and for searches. For exampleParticle.from_pdgid()
returns aParticle
instance whereasParticle.findall()
returns a list ofParticle
instances. While very powerful, this may be confusing at first. One could split functionality between a slightly differentParticle
class and aParticles
/ParticleTable
class.The text was updated successfully, but these errors were encountered: