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

Give firecrown.parameters.create a more descriptive name #333

Closed
tilmantroester opened this issue Oct 20, 2023 · 0 comments · Fixed by #334
Closed

Give firecrown.parameters.create a more descriptive name #333

tilmantroester opened this issue Oct 20, 2023 · 0 comments · Fixed by #334
Assignees

Comments

@tilmantroester
Copy link
Contributor

IMO, create is not a very descriptive name. Especially when used with from firecrown.parameters import create, having code in Updatables that looks like

def __init__(self):
    ...
    self.f_bar = create()

is not very self-explanatory, especially considering all the complexity that happens behind the scenes.
Calling it something like register_new_parameter might be helpful, since I imagine many users are not used to kind of __setattr__ magic that's being used here.

@vitenti vitenti self-assigned this Oct 20, 2023
@vitenti vitenti linked a pull request Oct 20, 2023 that will close this issue
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

Successfully merging a pull request may close this issue.

2 participants