ncoghlan commented 5 years ago

At time of filing, all boolean options are being handled such that passing even --user=False will still set the user flag.

5 proposes changing that so all false vales are handled as "don't pass the given argument", but that isn't good for cases like `--use-pep517` where it would be really nice to handle that as:

omitted or use_pep517=None -> no related option passed
use_pep517=True -> --use-pep517 option passed
use_pep517=False -> --no-use-pep517 option passed

One possible way to achieve that would be to make it so that all boolean options were handles as being tri-value (at the API level) from the start, such that:

some_option=True -> --some-option is added to the CLI (pip will report an error for some negative-only options, but we can live with that)
some_option=False -> --no-some-option is added to the CLI (pip will report an error for some positive-only options, but we can live with that)
some_option=None -> don't specifiy either positive or negative

Given that model, the wrapper would then also intercept all keywords starting with no_ and advise flipping them to their tri-state counterpart:

no_some_option=None -> ValueError recommending some_option=None
no_some_option=True -> ValueError recommending some_option=False
no_some_option=False -> ValueError recommending some_option=True

ncoghlan commented 5 years ago

Checking https://pip.pypa.io/en/stable/reference/pip_install/#options for any other design constraints:

-U is a short boolean option, but I'm inclined to actually disallow the use of short options in the Python API entirely (on the basis of "there should be one, and preferably only one, obvious way to do it", and the fact that "upgrade=True" is far less cryptic than "U=True"). Short/long options are a CLI convention because they allow multiple short options to be combined into a single CLI argument, which isn't relevant here. For now, I'll just disallow the "U=False" case and leave "U=True" alone.
--binary/--no-binary are actually filter conditions rather than a toggle pair, so there shouldn't be any special casing of the "no_" prefix when the value is being used as a CLI argument.

I'll amend #5 on that basis.

ncoghlan commented 5 years ago

5 has been updated to match the above description now. I didn't end up disallowing `U=False` explicitly - instead, it gets translated to `--no-U`, which `pip` will reject as an unknown option.

reynoldsnlp commented 5 years ago

Fixed by https://github.com/reynoldsnlp/pip_inside/pull/5

reynoldsnlp / pipster

Handle all boolean options as tri-state values #6

5 proposes changing that so all false vales are handled as "don't pass the given argument", but that isn't good for cases like `--use-pep517` where it would be really nice to handle that as:

5 has been updated to match the above description now. I didn't end up disallowing `U=False` explicitly - instead, it gets translated to `--no-U`, which `pip` will reject as an unknown option.

reynoldsnlp / pipster

Handle all boolean options as tri-state values #6

5 proposes changing that so all false vales are handled as "don't pass the given argument", but that isn't good for cases like --use-pep517 where it would be really nice to handle that as:

5 has been updated to match the above description now. I didn't end up disallowing U=False explicitly - instead, it gets translated to --no-U, which pip will reject as an unknown option.

5 proposes changing that so all false vales are handled as "don't pass the given argument", but that isn't good for cases like `--use-pep517` where it would be really nice to handle that as:

5 has been updated to match the above description now. I didn't end up disallowing `U=False` explicitly - instead, it gets translated to `--no-U`, which `pip` will reject as an unknown option.