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

Model info as function attribute #142

Closed
stestoll opened this issue Apr 5, 2021 · 3 comments · Fixed by #143
Closed

Model info as function attribute #142

stestoll opened this issue Apr 5, 2021 · 3 comments · Fixed by #143
Labels

Comments

@stestoll
Copy link
Collaborator

stestoll commented Apr 5, 2021

Currently, model info is returned by calling the model function without arguments, and an internal conditional statement distinguishes this call from the actual model call.

It would be cleaner (and more Pythonic) to use a function attribute for storing and providing these model metadata. See "User-defined functions" in the official Python doc.

Two possibilities:

  • Store the info dict as a function attribute, e.g. bg_hom3d.info = dict(...).
  • Store the key-value pairs from the info dict separately as function attributes, e.g. bg_hom3d.Parameters = ..., bg_hom3d.Units = ... etc.

The second possibility seems a lot cleaner. There is no need to introduce an additional dict layer as done in the first possibility.

In either case, we need to watch whether this structural change has any impact on performance when evaluating the model.

@mtessmer
Copy link
Collaborator

mtessmer commented Apr 5, 2021

Using a dictionary should allow for individual access through the info attribute so you should be able to call bg_hom3d.info.Parameters and have the best of both worlds.

@stestoll
Copy link
Collaborator Author

stestoll commented Apr 5, 2021

The function attributes are stored as a dict, so introducing another layer of dict via info might not be necessary. If one wants to the whole dict, one can do info = bg_hom3d.__dict__.

@mtessmer
Copy link
Collaborator

mtessmer commented Apr 5, 2021

That works as well; but, .__dict__ may also return any other (user-defined) attributes. An unlikely event that shouldn't disrupt any DeerLab internals if implemented in a safe way, but still one that can occur. There is still the issue of mutability too. You could make info a named tuple, but then we're back to the extra layer.

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

Successfully merging a pull request may close this issue.

3 participants