-
Notifications
You must be signed in to change notification settings - Fork 259
constraints/markers: fix union of multi constraints and thereby union of atomic multi markers resulting in invalid marker strings #875
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
Conversation
Reviewer's GuideUnion behavior for multi-constraints has been enhanced to return an AnyConstraint when there are no common elements and a single Constraint when there is exactly one common element, and corresponding tests for both constraint intersection and atomic marker union have been updated or added to reflect the new behavior. Class diagram for updated MultiConstraint.union behaviorclassDiagram
class BaseConstraint
class Constraint
class AnyConstraint
class MultiConstraint {
+constraints: List[Constraint]
+union(other: BaseConstraint) BaseConstraint
}
BaseConstraint <|-- Constraint
BaseConstraint <|-- AnyConstraint
BaseConstraint <|-- MultiConstraint
MultiConstraint --> Constraint : contains
%% Updated union method logic:
%% - If other is MultiConstraint, find common constraints
%% - If no common: return AnyConstraint
%% - If one common: return that Constraint
%% - Else: return MultiConstraint of common constraints
%% - If other is not Constraint: (not shown in this diff)
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:
- Please update the docstring of MultiConstraint.union to document that it can now return an AnyConstraint or a single Constraint when common constraints count is 0 or 1.
- Consider refactoring the common-length checks in union into early return guard clauses to simplify the control flow.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- Please update the docstring of MultiConstraint.union to document that it can now return an AnyConstraint or a single Constraint when common constraints count is 0 or 1.
- Consider refactoring the common-length checks in union into early return guard clauses to simplify the control flow.Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
… of atomic multi markers resulting in invalid marker strings
f4539e6 to
043460d
Compare
This PR contains the following updates: | Package | Update | Change | |---|---|---| | [poetry](https://github.com/python-poetry/poetry) ([changelog](https://python-poetry.org/history/)) | minor | `2.1.4` -> `2.2.1` | --- ### Release Notes <details> <summary>python-poetry/poetry (poetry)</summary> ### [`v2.2.1`](https://github.com/python-poetry/poetry/blob/HEAD/CHANGELOG.md#221---2025-09-21) [Compare Source](python-poetry/poetry@2.2.0...2.2.1) ##### Fixed - Fix an issue where `poetry self show` failed with a message about an invalid output format ([#​10560](python-poetry/poetry#10560)). ##### Docs - Remove outdated statements about dependency groups ([#​10561](python-poetry/poetry#10561)). ##### poetry-core ([`2.2.1`](https://github.com/python-poetry/poetry-core/releases/tag/2.2.1)) - Fix an issue where it was not possible to declare a PEP 735 dependency group as optional ([#​888](python-poetry/poetry-core#888)). ### [`v2.2.0`](https://github.com/python-poetry/poetry/blob/HEAD/CHANGELOG.md#220---2025-09-14) [Compare Source](python-poetry/poetry@2.1.4...2.2.0) ##### Added - **Add support for nesting dependency groups** ([#​10166](python-poetry/poetry#10166)). - **Add support for PEP 735 dependency groups** ([#​10130](python-poetry/poetry#10130)). - **Add support for PEP 639 license clarity** ([#​10413](python-poetry/poetry#10413)). - Add a `--format` option to `poetry show` to alternatively output json format ([#​10487](python-poetry/poetry#10487)). - Add official support for Python 3.14 ([#​10514](python-poetry/poetry#10514)). ##### Changed - **Normalize dependency group names** ([#​10387](python-poetry/poetry#10387)). - Change `installer.no-binary` and `installer.only-binary` so that explicit package names will take precedence over `:all:` ([#​10278](python-poetry/poetry#10278)). - Improve log output during `poetry install` when a wheel is built from source ([#​10404](python-poetry/poetry#10404)). - Improve error message in case a file lock could not be acquired while cloning a git repository ([#​10535](python-poetry/poetry#10535)). - Require `dulwich>=0.24.0` ([#​10492](python-poetry/poetry#10492)). - Allow `virtualenv>=20.33` again ([#​10506](python-poetry/poetry#10506)). - Allow `findpython>=0.7` ([#​10510](python-poetry/poetry#10510)). - Allow `importlib-metadata>=8.7` ([#​10511](python-poetry/poetry#10511)). ##### Fixed - Fix an issue where `poetry new` did not create the project structure in an existing empty directory ([#​10431](python-poetry/poetry#10431)). - Fix an issue where a dependency that was required for a specific Python version was not installed into an environment of a pre-release Python version ([#​10516](python-poetry/poetry#10516)). ##### poetry-core ([`2.2.0`](https://github.com/python-poetry/poetry-core/releases/tag/2.2.0)) - Deprecate table values and values that are not valid SPDX expressions for `[project.license]` ([#​870](python-poetry/poetry-core#870)). - Fix an issue where explicitly included files that are in `.gitignore` were not included in the distribution ([#​874](python-poetry/poetry-core#874)). - Fix an issue where marker operations could result in invalid markers ([#​875](python-poetry/poetry-core#875)). </details> --- ### Configuration 📅 **Schedule**: Branch creation - At any time (no schedule defined), Automerge - At any time (no schedule defined). 🚦 **Automerge**: Disabled by config. Please merge this manually once you are satisfied. ♻ **Rebasing**: Whenever PR becomes conflicted, or you tick the rebase/retry checkbox. 🔕 **Ignore**: Close this PR and you won't be reminded about this update again. --- - [ ] <!-- rebase-check -->If you want to rebase/retry this PR, check this box --- This PR has been generated by [Renovate Bot](https://github.com/renovatebot/renovate). <!--renovate-debug:eyJjcmVhdGVkSW5WZXIiOiI0MS42MC40IiwidXBkYXRlZEluVmVyIjoiNDEuNjAuNCIsInRhcmdldEJyYW5jaCI6Im1haW4iLCJsYWJlbHMiOltdfQ==--> Reviewed-on: https://git.walbeck.it/walbeck-it/docker-python-poetry/pulls/1588 Co-authored-by: renovate-bot <[email protected]> Co-committed-by: renovate-bot <[email protected]>
Resolves: python-poetry/poetry#10439
Summary by Sourcery
Fix union logic for multi constraints and atomic multi markers to properly return AnyConstraint/AnyMarker when there are no common elements and return a single constraint/marker when there is exactly one common element, accompanied by new tests to verify these behaviors.
Bug Fixes:
Tests: