-
Notifications
You must be signed in to change notification settings - Fork 32
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
Tree pruning #67
base: main
Are you sure you want to change the base?
Tree pruning #67
Conversation
Also need to write comments to describe how things work. |
353f6db
to
133db3c
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Approach looks great to me.
I'm a little concerned about the flexibility we're giving the provider here, but, I guess that's a ship that sailed a while back. 🙃
Forgot to mention: Please feel free to suggest alternative method names. I’m terrible at this. |
|
||
Both arguments are iterators yielding requirement objects. A boolean | ||
should be returned to indicate whether the two sets should be treated | ||
as matching. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This should also mention requirements_a
is the new state, and requirements_b
is from a known-to-fail state. This is important since the provider can implement range merging to return True when requirements_a
is a subset of requirements_b
to prune additional subtrees.
(The method name should probably change.)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
OTOH if requirements_a
is a superset of requirements_b
, this should be used to exclude requirements_b
from requirements_a
so the resolver can avoid visiting some subtrees. Maybe the interface should be defined to perform specifier merging instead.
This produces a ResolutionImpossible against
pip install prefect[all_extras] --only-binary :all:
in ~30s.Still much to work on, mainly error reporting seems wrong. I can’t make sense of the report from the
perfect
example. It contains manyprefect[all-extras]
entries (different versions), but does not actually show where the conflict is. But the approach itself seems to work.