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

Misnamed parameter "include_significant" on method Task.get_param_names #3167

Open
MartinHeimsoth-V opened this issue May 27, 2022 · 1 comment

Comments

@MartinHeimsoth-V
Copy link

The classmethod Task.get_param_names has either a really confusingly misnamed parameter include_significant, or the implementation has a bug.
Example:

>>> class T(Task):
...     significant = Parameter(significant=True)
...     insignificant = Parameter(significant=False)
... 
>>> T.get_param_names()
['significant']
>>> T.get_param_names(include_significant=True)
['significant', 'insignificant']

So, I'd suggest to rename the parameter to include_insignificant (insignificant), or to change the implementation that on default it only returns the insignificant parameters, which would be weird though.

@nayopu
Copy link

nayopu commented May 29, 2022

@MartinHeimsoth-V
I was wondering the same thing.

Regarding your suggestions,

So, I'd suggest to rename the parameter to include_insignificant (insignificant), or to change the implementation that on default it only returns the insignificant parameters, which would be weird though.

The former suggestion sounds better. I see the include_significant is just a typo and expected behavior is to return only significant parameters on default.
The later suggestion of changing default behavior is a major change that can cause some problems in existing codes.

@daveFNbuck
I saw you were involved in the latest revision of this method. Do you have any idea on the expected behavior?

I made a related pull-request (#3168) before seeing this issue.
This request is accidently equivalent to the former suggestion of renaming include_significant.

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

2 participants