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

confusing unpredicatable behaviour #129

Open
shimwell opened this issue Apr 14, 2021 · 1 comment
Open

confusing unpredicatable behaviour #129

shimwell opened this issue Apr 14, 2021 · 1 comment

Comments

@shimwell
Copy link
Collaborator

When specifying a Material() with Isotopes and a name the program will look up the name internally and find might find (if it exsits) the corresponding name.

Then the isotopes, density and other attributes will be replaced with the internal definition.

Perhaps there should be a check that prevents this

@eepeterson
Copy link

Generally with a call to a constructor like mat = nmm.Material(**kwargs) there shouldn't be any side effects that aren't clear to the user. The ability to create a material with no args (i.e. mat = nmm.Material()) should be feasible and have a sensible default (most likely all attributes None), but alternatively you could specify a minimum set of necessary kwargs to define an nmm material.

The other source of unpredictable behavior surrounds setters and getters. The setters for material.openmc_material and Material.mcnp_material and others should be removed because setting them does not create a consistent internal state for the Material object and subsequent exports will not produce what the user expects. The getters for these materials should also compute and return them on the fly rather than storing them in private attributes.

Generally, there should only be one way to set an attribute and the side effects of that should be minimized and carefully documented.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants