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

Make model.config.par_names() a property? #1700

Closed
1 task done
alexander-held opened this issue Nov 15, 2021 · 2 comments · Fixed by #2027
Closed
1 task done

Make model.config.par_names() a property? #1700

alexander-held opened this issue Nov 15, 2021 · 2 comments · Fixed by #2027
Assignees
Labels
feat/enhancement New feature or request user request Request coming form a pyhf user

Comments

@alexander-held
Copy link
Member

Summary

Currently model.config.par_names() takes no arguments and returns a list of parameter names. The behavior slightly differs from model.config.channels and model.config.samples, which are properties. For consistency, it may be useful to turn par_names into a property as well.

Additional Information

model.config.par_names() used to take an argument to set formatting, which required it to be a function. Since that has been dropped in the meantime, this opens up the possibility of using @property.

I don't know whether both par_names and par_names() can be supported simultaneously, which would allow to not break existing workflows with par_names(), but it may be better to only have a single API regardless.

Code of Conduct

  • I agree to follow the Code of Conduct
@alexander-held alexander-held added feat/enhancement New feature or request needs-triage Needs a maintainer to categorize and assign labels Nov 15, 2021
@alexander-held
Copy link
Member Author

Similarly to this, suggested_init, suggested_bounds and suggested_fixed currently take no arguments either. One other consideration is that maybe some of these parts of the API are meant to eventually support kwargs, in which case switching to a property would not be useful.

@matthewfeickert
Copy link
Member

Similarly to this, suggested_init, suggested_bounds and suggested_fixed currently take no arguments either. One other consideration is that maybe some of these parts of the API are meant to eventually support kwargs, in which case switching to a property would not be useful.

I like this idea, but would table it until v0.8.0. So I've moved this to Issue #2028.

@matthewfeickert matthewfeickert removed the needs-triage Needs a maintainer to categorize and assign label Sep 23, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feat/enhancement New feature or request user request Request coming form a pyhf user
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants