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

Rethink the settings attribute #2861

Open
jgostick opened this issue Nov 20, 2023 · 0 comments
Open

Rethink the settings attribute #2861

jgostick opened this issue Nov 20, 2023 · 0 comments

Comments

@jgostick
Copy link
Member

jgostick commented Nov 20, 2023

I hate the way that the settings attribute/functionality worked out. I was trying to solve a problem we had where "developer X" was storing objects as settings instead of a reference to the object. This made me think we had to create a "robust" settings attribute that protected each item to maintain the correct datatype. Around the same time dataclasses were becoming popular, so I tried to mimic those. The final result was a complicated mess but it actually worked. However, it turns out that the pickle module does not work, which is a problem since this is our main suggested method for storing work.

So, I think the settings attribute needs to be redesigned. The following is a wish list of features, just to remind myself of what I was going for at the time. We might not necessarily still want all of these:

  • Protected data types so users cannot write a numpy array when a string is required
  • If the data was an iterable, then it TOO need to prevent other things from being written into it (like keep it all ints or floats).
  • Inherited settings from super classes, so if Transport has settings like 'rtol', then FickianDiffusion will inherit this setting, then add its own.
  • Docstrings are also easily inherited so the settings from higher levels are explained. We are currently using docrep for this. I put a lot of weight on the docstrings...I really like when the docs are available in the IDE so people can see the settings and their purpose easily.
  • Keep the settings attribute name space clean. Using things like traits and pydandic results in dozens of their class-specific methods polluting the namespace (like alg.settings.<cruft>). Using a dictionary also gives a lot of cruft, so maybe the setting should be stored IN the dictionary instead of as attributes?
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

1 participant