-
Notifications
You must be signed in to change notification settings - Fork 261
simplify extra markers 3 #847
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
simplify extra markers 3 #847
Conversation
Reviewer's Guide by SourceryThis pull request simplifies extra markers by implementing optimizations for unions of multi markers and intersections of union markers. The changes involve modifying the intersect method in Sequence diagram for intersecting UnionConstraint with another UnionConstraintsequenceDiagram
participant UC1 as UnionConstraint (A or B)
participant UC2 as UnionConstraint (A or B or C)
UC1->>UC2: intersect(UC1)
UC2->>UC2: our_constraints.issubset(their_constraints) is true
UC2-->>UC1: return self (UC1)
participant UC3 as UnionConstraint (A or B)
participant UC4 as UnionConstraint (A or B or C)
UC4->>UC3: intersect(UC4)
UC3->>UC3: their_constraints.issubset(our_constraints) is true
UC3-->>UC4: return other (UC4)
Sequence diagram for intersecting UnionConstraint with a MultiConstraintsequenceDiagram
participant UC as UnionConstraint (A or B)
participant MC as MultiConstraint (A and D)
UC->>MC: intersect(UC)
loop for each constraint in UC
UC->>MC: intersection = our_constraint
loop for each constraint in MC
UC->>MC: intersection = intersection.intersect(their_constraint)
end
end
UC-->>MC: return new UnionConstraint
Updated class diagram for MultiConstraintclassDiagram
class MultiConstraint {
-constraints: list[BaseConstraint]
+intersect(other: BaseConstraint): BaseConstraint
+union(other: BaseConstraint): BaseConstraint
+ __bool__() : bool
}
MultiConstraint -- BaseConstraint : contains
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
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.
Hey @radoering - I've reviewed your changes - here's some feedback:
Overall Comments:
- It might be helpful to add a comment explaining the logic behind the simplification rules.
Here's what I looked at during the review
- 🟢 General issues: all looks good
- 🟢 Security: all looks good
- 🟡 Testing: 1 issue found
- 🟢 Complexity: all looks good
- 🟢 Documentation: all looks good
Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
With this change unions of multi markers and intersections of union markers where one marker is the subset of the other are simplified.
44fd5e7
to
c3d0c9f
Compare
With this change unions of multi markers and intersections of union markers where one marker is the subset of the other are simplified.
Related to: python-poetry/poetry#10195
Summary by Sourcery
Tests: