search

sarugaku / resolvelib

Resolve abstract dependencies into concrete ones
ISC License
142 stars 31 forks source link

Pre-compute backtrack cause names #92

Closed notatallshaw closed 3 years ago

notatallshaw commented 3 years ago

As discussed here: https://github.com/pypa/pip/pull/10621#issuecomment-954821065

notatallshaw commented 3 years ago

Probably should rename backtrack_causes to backtrack_names but this is just a minimal PR for now.

uranusjr commented 3 years ago

This boils down to if we want to expose the full conflict cause to the provider. pip only makes use of the identifiers, but would it be potentially useful to have access to all the additional information? resolvelib does have other existing users, pip is only the most prominent one receiving the most contributions, so unless we can be sure the additional information is not useful, I'd rather keep it available and push _cause_to_name into pip instead. If both the list of identifiers and the additional information are useful, we should devise a data structure to make the calculation cheap for both use cases.

notatallshaw commented 3 years ago

I'd rather keep it available and push _cause_to_name into pip instead.

So the issue is this is efficient to calculate once after each backtrack, and pip I think does not know when a backtrack occurs.

If it's possible to run this in pip only once after a backtrack without significantly increasing the complexity then that removes the reason for this PR.

notatallshaw commented 3 years ago

If both the list of identifiers and the additional information are useful, we should devise a data structure to make the calculation cheap for both use cases.

Currently only the identifiers are useful. I picked the current data structure with the additional information only because it was a data structure already being passed around so it felt minimally disruptive to me.

If a new data structure is created so the provider can calculate the names and store it on the data structure it's very important that it be mutable so the set of names can be replaced entirely and the id updated allowing the approach proposed in https://github.com/pypa/pip/pull/10621 to actually work.

notatallshaw commented 3 years ago

Okay, I've created a new PoC PR that attempts to address @uranusjr comments while being able to preserve what I think is required to make this work without it becoming too complex: https://github.com/sarugaku/resolvelib/pull/93

Closing this PR as I don't think it will get any consensus.