Feedback on warn-before-installation

[pradyunsg]

This issue is for collecting feedback on the warn before installation (i.e. warn after resolution) behaviour currently implemented in resolver/warn-after-resolution branch on this repository.

---

One known limitation of it is that it can't currently deal with extras since information about installed extras is not stored in the installation metadata. [1] That's something for later though, Sorry! :)

Open Questions:

• Is the information useful where it is presented?
• Are the messages worded in a clear enough manner?
• Is it too noisy?
  • Is --no-warn-conflicts good enough here?

---

To checkout this branch, I suggest you create a virtualenv (you use virtualenv or venv right?) and use the following to install this branch:

pip install --ignore-installed https://github.com/pradyunsg/pip/archive/resolver/warn-after-resolution.zip

[rgommers]

Is e81b602 really disallowing non-wheel build dependencies? If so, that doesn't sound quite right - build dependencies with compiled extensions (e.g. numpy is a common build dependency due to its C API) will likely fail on non-standard platforms.

[rgommers]

For who is looking for a diff: the "Files changed" tab of pypa/pip@master...pradyunsg:resolver/warn-after-resolution

[rgommers]

Can't try it out right now, but from reading the code things look good to me overall. One thing I'm not sure the --no-warn-conflicts is necessary, from a UX perspective it's a bit odd. Can you not capture this under a general verbosity level switch?

[pradyunsg]

Is e81b602 really disallowing non-wheel build dependencies? If so, that doesn't sound quite right - build dependencies with compiled extensions (e.g. numpy is a common build dependency due to its C API) will likely fail on non-standard platforms.

Hmm... I think a better place to discuss this is pypa#4802?

Can you not capture this under a general verbosity level switch?

I could but I'm not sure how that would work. Mostly, I don't see too many people using --quiet in the wild (and prefer it that way) and this behaviour more useful when it's opt-out. Otherwise, everyone could just run pip check.

[rgommers]

Mostly, I don't see too many people using --quiet in the wild (and prefer it that way) and this behaviour more useful when it's opt-out.

Yeah, not many people use --quiet and these warnings seem very relevant to users, that why I'm questioning whether you need the opt-out at all. Why not just always enable this?

[rgommers]

Hmm... I think a better place to discuss this is pypa#4802?

Indeed, thanks. I have a bad memory it seems:(

[pradyunsg]

Why not just always enable this?

Hmm... The initial thought was that it might not be able to identify the right conflicts due to the missing extras information and I was concerned if it's too noisy in some cases.

I didn't actually give it a second thought and I guess it'll only miss out on conflicts, not introduce new ones. The noise part is something I'd like to hear about here.

Happy to reduce the number of knobs. =)

[MarSoft]

Not sure if my question fits this topic, but will post it here:
I have just tested the branch in question with my problem pypa#4653. Namely, looks like this new branch still doesn't properly handle extras, so that only first set specified is actually used.
This still doesn't work:
pip install mypackage[extra1] mypackage[extra2,extra3] - this will install only mypackage[extra1].

[pradyunsg]

Oops! I thought I'd responded @MarSoft. Sorry!

So, the issue you're pointing out, needs the proper resolver to be brought in to fix. This branch does not in fact change any code in the dependency resolution code in pip -- that's what I'll change next. :)

[pradyunsg]

/ping @pfmoore @xavfernandez @dstufft

[pfmoore]

Not sure what feedback you want from me? I haven't tried out the branch, and I don't see any description of what it does compared to pip master. The phrase "warn before installation" doesn't really give me any feel for what it's doing.

[pradyunsg]

Not sure what feedback you want from me?

I'd like to know if you think this change is a useful one.

I haven't tried out the branch, and I don't see any description of what it does compared to pip master. The phrase "warn before installation" doesn't really give me any feel for what it's doing.

a picture is worth a thousand words

Essentially, this branch adds code to print that line in red. Basically, a warning whenever pip selects an incorrect set of packages for some reason, printed before the actual installation code is run.

[pfmoore]

But there's no "Are you sure?" prompt? I guess I don't see how printing that information can be bad, but by the time the user sees it there's nothing much they can do about it. And I doubt we can reasonably offer any advice in the message as to how to fix any damage...

So I don't see any harm in it, but doubt it'll actually help much in practice, either.

[MarSoft]

Probably we could show message and then delay further operations for e.g. 5 seconds, to give the user possibility to cancel. пт, 16 февр. 2018 г., 17:11 Paul Moore <[email protected]>:

…

But there's no "Are you sure?" prompt? I guess I don't see how printing that information can be bad, but by the time the user sees it there's nothing much they can do about it. And I doubt we can reasonably offer any advice in the message as to how to fix any damage... So I don't see any harm in it, but doubt it'll actually help much in practice, either. — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#1 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AAKc6xbBsadzrD2F_wnHJckObSKnENfIks5tVYyfgaJpZM4Ruj7O> .

[pfmoore]

I'm not too keen on that - a delay won't help people who start a long-ish install and then work on something else, and it will frustrate people who do want to continue. OTOH, an "Are you sure?" prompt will potentially cause issues with unattended installs such as CI (we've had this issue with people who prompt in setup.py`). UI design is hard :-(

On reflection, are we sure that the benefit of getting such a message is worth it?

Is this something that "having a proper resolver" would fix? It's "a warning whenever pip selects an incorrect set of packages for some reason" - isn't the ultimate goal for pip to not select an incorrect set of packages? The message doesn't even suggest what the user can do to fix the issue...

[pradyunsg]

I guess I won't mind adding a (short 2s-3s) delay if others are fine with doing that.

---

On reflection, are we sure that the benefit of getting such a message is worth it?

As I see it, currently pip just breaks the dependency requirements without any sign that it has done so - leading to those hard to debug issues. I see this as an improvement in that regard, at least now pip's telling you it broke stuff when it does it.

Functionally, this is running pip check on what packages would look like after installation but before actually installing them. Running pip check after installations, is not something that I've not seen too many people doing today.

The message doesn't even suggest what the user can do to fix the issue...

Agreed.

The "fix" is to do another pip --upgrade run, additionally specifying the constraints that were broken and the top-level package which may again print some more constraints which means another run -- it's not really very nice or clean.

I doubt we can reasonably offer any advice in the message as to how to fix any damage...

+1

Is this something that "having a proper resolver" would fix? isn't the ultimate goal for pip to not select an incorrect set of packages?

Yes it is. The motivation behind this is that if we end up doing a pip release before I find the time to bring in zazo to pip, this is an low-effort, okay, short-term-only improvement over the status quo. :)

(we've had this issue with people who prompt in setup.py)

Another one of those unreleased fixes. :(

PS: This is why I wanted your feedback. ^>^

[pfmoore]

I guess I won't mind adding a (short 2s-3s) delay if others are fine with doing that.

I don't really want to add a delay - "quick, hit Ctrl-C if you don't like what I just said" is a really bad UI, IMO. And interactive prompts are problematic for scripted situations (that can be worked around with --yes style options, but things then get out of hand for a temporary fix).

I see this as an improvement in that regard, at least now pip's telling you it broke stuff when it does it.

Agreed. You've convinced me this is worth it.

PS: This is why I wanted your feedback. ^>^

Glad I'm helping :-)

[pradyunsg]

Oh, and, I didn't mention -- I added an option to allow the user to silence these errors from being printed.

[pradyunsg]

I think pypa#5000 is ready to go. Any other concerns or feedback on the functionality?

[pfmoore]

No, I'm OK with the idea. I won't have time to review that PR, I'm afraid, so I'll leave it to you to decide if you want to merge it. I'm fine with the functionality going into pip 10, though.

(One minor thing I want to confirm - you didn't say so explicitly but from a quick scan of the code I guess you didn't add a delay or prompt - which is good, I would not want this to go into pip 10 with a delay/prompt, that decision would need some more time for review).

[pradyunsg]

There's no prompt. There's a new option --no-warn-conflicts that may be passed to prevent pip from printing these new warnings, if someone does not find them useful.

[pfmoore]

Cool, that's what I understood. So yeah, I'm fine with this.

[pradyunsg]

Closing this -- this feature/behavior has released as a part of pip 10. :)