Made a metaclass for building surface classes from specifications to handle almost all surface types. #940
Made a metaclass for building surface classes from specifications to handle almost all surface types. #940
Conversation
|
TODO:
|
tjlaboss
left a comment
There was a problem hiding this comment.
Overall a useful PR that replaces a lot of verbose code with some much-needed abstraction. A few nitpicks, a few small questions, and one big question.
How do we want to handle the parameter surface_type going forward?
Example: Consider ZCylinder. In the current implementation, this is a CylinderOnAxis on the CZ and not a C/Z.
>>> ZCylinder(number=1).mcnp_str()
montepy.exceptions.IllegalState: Surface: 1 does not have a surface type set.
>>> ZCylinder(number=1, surface_type="C/Z").mcnp_str()
ValueError: ZCylinder must be a surface of type: ['CZ']It seems that this should do one of the following:
- Delete the parameter
surface_type, since it must always beCZ. - Support
C/Zas an acceptablesurface_type - Delete the parameter
surface_type, and set the attribute toCZif no coordinates are specified, or elseC/Z: likeopenmc.ZCylinder.
That does not have to be addressed in this PR, as the interface change might necessitate a major release. A followon issue should be opened if not addressed here.
My initial thought is that we restrict the accepted values for As for As for I'm thinking:
Sound good @tjlaboss? |
I like the idea of this. The one thought I had was: is using |
I went ahead with keeping them as |
yes. |
|
@tjlaboss I deprecated all of those classes now. |

Pull Request Checklist for MontePy
Description
This project started as a fun weekend project of me asking copilot for a prototype. After seeing what it made I realized it was actually pretty straight forward, so I made the metaclass myself (mostly). Then I bugged Claude to write all of the specifications. This was partially an experiment in having AI as an unpaid intern.
This created a MetaClass
_SurfaceClassFactorythat can create aSurfaceclass from a minimal specification. The specification is given by@dataclasses(I wanted to play around with them) that primarily specifies what each surface parameter/constant means. These specifications are then used to make new properties that are attached to the class (vianamespace) prior to the class being created, since it is a MetaClass after all.In the documentation I gave guidance that users should start using
ZCylinderinstead ofCylinderOnAxis, but that the latter won't be deprecated. Is this reasonable guidance?Fixes #502, fixes #935
Questions:
z,a, etc., be lower or upper_case?General Checklist
blackversion 25 or 26.LLM Disclosure
Are you?
Were any large language models (LLM or "AI") used in to generate any of this code?
Documentation Checklist
First-Time Contributor Checklist
pyproject.tomlif you wish to do so.Additional Notes for Reviewers
Ensure that:
📚 Documentation preview 📚: https://montepy--940.org.readthedocs.build/en/940/