sarugaku / resolvelib

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

Implement dedicated causes object #93

Closed notatallshaw closed 2 years ago

notatallshaw commented 2 years ago

This is proof of concept to address issues raised in https://github.com/sarugaku/resolvelib/pull/92#issuecomment-955139430

Derived information can then be calculated and cached as needed. In particular the downstream provider can run identifier in backtrack_causes.names where names is a property that calculates the underlying information lazily and only has to be re-calculated if the underlying causes are changed.

I'm trying to see if this approach would have consensuses, several things in the code would need fixing before merging.

notatallshaw commented 2 years ago

A nice thing about this approach is it allows the downstream library to be decoupled from the internal structure of the causes. Maybe helpful?

uranusjr commented 2 years ago

I think I like this approach the best, but we need to go a bit further down the decoupling path. Taking things like pypa/pip#10478 into account, perhaps we should take a similar approach to Requirement and Candidate and add an interface in Provider custom Cause object logic? (We can provide a default implementation if that’s a good idea.)

notatallshaw commented 2 years ago

That makes sense to me, I will take a look at how Requirement and Candidate are implemented and mirror that approach.

hswong3i commented 2 years ago

So #97 and 1.0.0 should now depends on this PR?

uranusjr commented 2 years ago

Yes.

Technically not this PR specifically, but the implementation of a new provider method that allows creating a custom conflict cause object that the resolver will modify and pass back to other provider hooks. That may happen in this PR, another PR, or a combination of several.

hswong3i commented 2 years ago

Seems a huge workload for Cause, which may need more than a month for implement it precisely… Maybe this breaking change could go into 2.0.0? Or just implement the API interface changes now for 1.0.0 without indeed and perfect implementation, yet?

jbylund commented 2 years ago

Technically not this PR specifically, but the implementation of a new provider method that allows creating a custom conflict cause object that the resolver will modify and pass back to other provider hooks. That may happen in this PR, another PR, or a combination of several.

Isn't that what #96 is? Using that approach a provider could pass make_causes_obj_from_raw_causes if it wanted to (though my preference would be for just set of strings).

uranusjr commented 2 years ago

Yes except I want it to be a provider method and create the entire cause object, not modify it.

notatallshaw commented 2 years ago

Not had any time to work on this this last month. However I expect to have some free time during the holidays. I'm probably going to create a new PR and close this one, implementing it like Requirement and Candidate the way @uranusjr suggests.

I don't feel any ownership over doing the work 😉, so if anyone else wants to take the approach and implement it themselves feel free to, otherwise I will come back with an update later this month.

notatallshaw commented 2 years ago

I think I like this approach the best, but we need to go a bit further down the decoupling path. Taking things like pypa/pip#10478 into account, perhaps we should take a similar approach to Requirement and Candidate and add an interface in Provider custom Cause object logic? (We can provide a default implementation if that’s a good idea.)

I've addressed these comments in a new PR: https://github.com/sarugaku/resolvelib/pull/99 and an accompanying pip PR: https://github.com/pypa/pip/pull/10732