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

option descriptions regarding default value and behaviour can be inconsistent/misleading (but does it really matter?) #885

Open
migueldiascosta opened this issue Jan 8, 2024 · 1 comment

Comments

@migueldiascosta
Copy link
Member

following up on a conversation with @Flamefire in easybuilders/easybuild-easyblocks#3065:

            'pip_verbose': [None, "Pass --verbose to 'pip install' (if pip is used). "
                                  "Defaults to 'True' if the EB option --debug is used.", CUSTOM],

MC:

nitpicking but this isn't quite right, is it? (using --debug doesn't change the value of pip_verbose)

AG:

Technically you are right. It is rather the effect of this option that is enabled by default.
I don't think this is an issue as I can't see where the distinction matters and this is shorter in expressing what it does to users: If set to True --verbose is used. If set to False --verbose is not used. If unset --verbose is used if --debug is used. E.g. your wording would miss-describe the "False" case
And if we want to be really strict about this then we'd need to change either the implementation or other descriptions, e.g. "Defaults to '.' for unpacked sources or the first source file specified", "Enabled by default when pip_ignore_installed=True", "Defaults to 'True' except for whl files", ...

MC:

fair enough. "Enabled by default", in that second example, is better than "Defaults to 'True'" though (if one interprets that what's enabled by default is the passing of --verbose, not pip_verbose itself)

AG:

I was thinking back and forth:
"Defaults to True" makes it clearer to the user (only observing effects, not internal state): If not set the value will be True which means --verbose is used.
But it is confusing to developers: The default is technically still "None", not True
"Enabled by default" might be clearer to developers that this option isn't changed although it could still be interpreted this way.
Additionally it might be confusing to users misinterpreting that --debug overwrites this (user set value) unless "something else" is done.
So both have drawbacks hence I guess it doesn't matter. Maybe we should rather be consistent and change the others to either way. Used your suggestion here for now

MC

You're right.
(what about actually setting, as soon as possible, pip_verbose = True when --debug is used? Still not quite correct that it is the default, but at least a developer thinking about the internal state (after the value is set...) wouldn't make any wrong assumptions)
(...) will (...) open a separate issue about how to be consistent about this in general

@Flamefire
Copy link
Contributor

(what about actually setting, as soon as possible, pip_verbose = True when --debug is used? Still not quite correct that it is the default, but at least a developer thinking about the internal state (after the value is set...) wouldn't make any wrong assumptions)

This is possible but I'm not sure if there could be unintended side effects: Now the real default value, i.e. None (== "choose automatically") will be hidden. I imagine a script checking to remove superflous EC options can get confused by that.

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