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

Consumer passes in a prepare method which is called over backtrack causes #96

Open
wants to merge 3 commits into
base: main
Choose a base branch
from

Conversation

jbylund
Copy link
Contributor

@jbylund jbylund commented Nov 1, 2021

Add a prepare attribute to the Resolution object which is called on backtrack causes before passing that to _get_preference. This allows providers to operate over a processed version of backtrack_causes without having to implement a cache consumer side (which can be "yucky").

@jbylund
Copy link
Contributor Author

jbylund commented Nov 1, 2021

@notatallshaw this is the other possibility I mentioned. Let me know if you think this would mesh well with your ideas for backtracking in the future?

@notatallshaw
Copy link
Collaborator

notatallshaw commented Nov 1, 2021

This approach LGTM if maintainers want the logic of processing the causes to remain on the downstream library side.

And I continue to really prefer avoiding a cache that depends on the ids of mutable objects (even if they are carefully used).

I guess let's just see what maintainers think.

@uranusjr
Copy link
Member

I think #93 would supercede this now?

@jbylund
Copy link
Contributor Author

jbylund commented Nov 17, 2021

I think #93 would supercede this now?

My opinion is that this option would allow for:

  • the possibility of a simpler solution making use of stdlib collection types - class is reduced to a function which returns a set
  • more separation/flexibility between different consumers of resolvelib - users that might care about the order of conflicts (I don't even know if that could theoretically matter to someone) could choose the passthrough method

@uranusjr
Copy link
Member

Sounds like a provider function is also a fit for those reasons? If the consumer wishes, it can simply implement the function to return a set, or whatever built-in or custom type that suits the need.

@jbylund
Copy link
Contributor Author

jbylund commented Nov 18, 2021

Sounds like a provider function is also a fit for those reasons? If the consumer wishes, it can simply implement the function to return a set, or whatever built-in or custom type that suits the need.

But I think we need this "hook" to allow the provider to inject such a function?

@jbylund jbylund marked this pull request as ready for review December 20, 2021 23:51
@jbylund
Copy link
Contributor Author

jbylund commented Dec 20, 2021

The pip side of this change would be this.

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

Successfully merging this pull request may close these issues.

3 participants