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

change default_model_config form @property to @staticmethod #225

Closed
michaelraczycki opened this issue Aug 3, 2023 · 2 comments · Fixed by #235
Closed

change default_model_config form @property to @staticmethod #225

michaelraczycki opened this issue Aug 3, 2023 · 2 comments · Fixed by #235
Labels
good first issue Good for newcomers

Comments

@michaelraczycki
Copy link
Collaborator

Current design is shorter for usage inside the classes themselves, but when it comes to end user experience it forces initialization of the model class to take a look at customizable variables

@michaelraczycki michaelraczycki added the good first issue Good for newcomers label Aug 3, 2023
@pdb5627
Copy link
Contributor

pdb5627 commented Aug 23, 2023

This looks like something I could do, though I'd like to ask some clarification. It seems that starting in Python 3.11, @classmethod and @property cannot be chained. If the method is changed from a property to a class method, would you like it renamed from a noun (default_model_config) to a verb (e.g. get_default_model_config)? Or define a classproperty decorator so it can stay a property? Otherwise it's as straightforward as adding parentheses to references.

@michaelraczycki
Copy link
Collaborator Author

Hello @pdb5627, we don't want to chain the decorators, but only replace it (@property->@staticmethod). The main purpose of it is to remove the necesity of initialization of a model to see the default values from the user's standpoint. Yes, I think that the change to a verb should be done, but also it would require searching through the LinearModel and tests to replace it there

pdb5627 added a commit to pdb5627/pymc-experimental that referenced this issue Aug 25, 2023
Fixes pymc-devs#225. Since the method changed from a property to
a method, it was renamed from a noun to a verb. Both
`default_model_config` and `default_sampler_config` were
changed.
twiecki pushed a commit that referenced this issue Sep 1, 2023
* Change default_*_config from @Property to @staticmethod

Fixes #225. Since the method changed from a property to
a method, it was renamed from a noun to a verb. Both
`default_model_config` and `default_sampler_config` were
changed.

* Fix to use staticmethod instead of classmethod
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
good first issue Good for newcomers
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants