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

Simplify marker simplification #530

Merged
merged 7 commits into from
Dec 18, 2022

Conversation

dimbleby
Copy link
Contributor

per the discussion from #528 (comment), try to use reduction to DNF and CNF as much as possible to achieve marker simplifications, in the hope that:

  • it's more general than the collection of heuristics we currently have
  • it simplifies the code

I think this probably is an improvement - and the test coverage is pretty decent here, so there's reason to think that it doesn't break things too badly. But more opinion is certainly wanted.

@dimbleby dimbleby force-pushed the simplify-marker-simplification branch 3 times, most recently from 01eb695 to f9f5536 Compare November 27, 2022 17:01
src/poetry/core/version/markers.py Outdated Show resolved Hide resolved
src/poetry/core/version/markers.py Outdated Show resolved Hide resolved
@dimbleby dimbleby force-pushed the simplify-marker-simplification branch 4 times, most recently from 17740e4 to 984ebfa Compare November 29, 2022 19:40
@dimbleby dimbleby force-pushed the simplify-marker-simplification branch from 6be392d to 0a6d286 Compare November 29, 2022 20:39
@dimbleby
Copy link
Contributor Author

dimbleby commented Nov 29, 2022

Reckon I'm done fiddling with this now, I'm pretty happy with the much-reduced set of heuristic marker simplifications being largely replaced by more systematic cnf() / dnf()

remaining downstream failure is a benign rearrangement of a parsed marker per the final commit, if y'all are minded to merge this it's easy enough to fix up

@dimbleby dimbleby force-pushed the simplify-marker-simplification branch 6 times, most recently from 9f92002 to 3cee6d9 Compare November 30, 2022 20:14
dimbleby added a commit to dimbleby/poetry that referenced this pull request Dec 3, 2022
dimbleby added a commit to dimbleby/poetry that referenced this pull request Dec 3, 2022
@dimbleby dimbleby force-pushed the simplify-marker-simplification branch from b18e462 to 913c651 Compare December 3, 2022 13:37
dimbleby added a commit to dimbleby/poetry that referenced this pull request Dec 10, 2022
radoering pushed a commit to dimbleby/poetry that referenced this pull request Dec 11, 2022
@dimbleby dimbleby force-pushed the simplify-marker-simplification branch from 16dce60 to 5f22121 Compare December 11, 2022 18:22
@radoering
Copy link
Member

I added a general intersection and union function as proposed in python-poetry/poetry#7173 (comment) and moved some duplicated code in there. What do you think about it?

@radoering radoering force-pushed the simplify-marker-simplification branch from a739a08 to 7086f11 Compare December 17, 2022 09:19
@dimbleby
Copy link
Contributor Author

Fine by me.

I considered suggesting moving these to staticmethods on the BaseMarker so that callers would call eg BaseMarker.union(*markers). If the BaseMarker were ever so slightly less awkwardly named - eg just Marker would do - then I might favour that. But as it stands, I judge that would make calling code more ugly, rather than less ugly.

(And I don't think it's worth renaming the BaseMarker just for this)

don't get too hung up on symmetry for union and intersection
dimbleby and others added 5 commits December 18, 2022 06:58
Exploit the same cnf / dnf trick that is used in marker union
The potential downside of the approach in this series of commits is that
cnf() and dnf() can, in the worst case, be exponentially expensive.

eg `cnf((a1 and b1) or (a2 and b2) or ... (an and bn))` generates 2^n
intersections.

In practice, markers on python packages seem not to get long enough for
this to be an issue.

Deduplicating the markers on unions and intersections is the lowest of
low-hanging fruit in mitigating this.
…f markers, and move duplicated code into these functions
@radoering radoering force-pushed the simplify-marker-simplification branch from 7086f11 to 744e720 Compare December 18, 2022 05:58
@sonarcloud
Copy link

sonarcloud bot commented Dec 18, 2022

Kudos, SonarCloud Quality Gate passed!    Quality Gate passed

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 0 Code Smells

No Coverage information No Coverage information
No Duplication information No Duplication information

@radoering radoering enabled auto-merge (squash) December 18, 2022 06:03
@radoering radoering merged commit 5b361d7 into python-poetry:main Dec 18, 2022
@dimbleby dimbleby deleted the simplify-marker-simplification branch December 18, 2022 11:13
mwalbeck pushed a commit to mwalbeck/docker-python-poetry that referenced this pull request Feb 28, 2023
This PR contains the following updates:

| Package | Update | Change |
|---|---|---|
| [poetry](https://python-poetry.org/) ([source](https://github.com/python-poetry/poetry), [changelog](https://python-poetry.org/history/)) | minor | `1.3.2` -> `1.4.0` |

---

### Release Notes

<details>
<summary>python-poetry/poetry</summary>

### [`v1.4.0`](https://github.com/python-poetry/poetry/blob/HEAD/CHANGELOG.md#&#8203;140---2023-02-27)

[Compare Source](python-poetry/poetry@1.3.2...1.4.0)

##### Added

-   **Add a modern installer (`installer.modern-installation`) for faster installation of packages and independence from pip** ([#&#8203;6205](python-poetry/poetry#6205)).
-   Add support for `Private ::` trove classifiers ([#&#8203;7271](python-poetry/poetry#7271)).
-   Add the version of poetry in the `@generated` comment at the beginning of the lock file ([#&#8203;7339](python-poetry/poetry#7339)).
-   Add support for `virtualenvs.prefer-active-python` when running `poetry new` and `poetry init` ([#&#8203;7100](python-poetry/poetry#7100)).

##### Changed

-   **Deprecate the old installer, i.e. setting `experimental.new-installer` to `false`** ([#&#8203;7358](python-poetry/poetry#7358)).
-   Remove unused `platform` field from cached package info and bump the cache version ([#&#8203;7304](python-poetry/poetry#7304)).
-   Extra dependencies of the root project are now sorted in the lock file ([#&#8203;7375](python-poetry/poetry#7375)).
-   Remove upper boundary for `importlib-metadata` dependency ([#&#8203;7434](python-poetry/poetry#7434)).
-   Validate path dependencies during use instead of during construction ([#&#8203;6844](python-poetry/poetry#6844)).
-   Remove the deprecated `repository` modules ([#&#8203;7468](python-poetry/poetry#7468)).

##### Fixed

-   Fix an issue where an unconditional dependency of an extra was not installed in specific environments ([#&#8203;7175](python-poetry/poetry#7175)).
-   Fix an issue where a pre-release of a dependency was chosen even if a stable release fulfilled the constraint ([#&#8203;7225](python-poetry/poetry#7225), [#&#8203;7236](python-poetry/poetry#7236)).
-   Fix an issue where HTTP redirects were not handled correctly during publishing ([#&#8203;7160](python-poetry/poetry#7160)).
-   Fix an issue where `poetry check` did not handle the `-C, --directory` option correctly ([#&#8203;7241](python-poetry/poetry#7241)).
-   Fix an issue where the subdirectory information of a git dependency was not written to the lock file ([#&#8203;7367](python-poetry/poetry#7367)).
-   Fix an issue where the wrong Python version was selected when creating an virtual environment ([#&#8203;7221](python-poetry/poetry#7221)).
-   Fix an issue where packages that should be kept were uninstalled when calling `poetry install --sync` ([#&#8203;7389](python-poetry/poetry#7389)).
-   Fix an issue where an incorrect value was set for `sys.argv[0]` when running installed scripts ([#&#8203;6737](python-poetry/poetry#6737)).
-   Fix an issue where hashes in `direct_url.json` files were not written according to the specification ([#&#8203;7475](python-poetry/poetry#7475)).
-   Fix an issue where poetry commands failed due to special characters in the path of the project or virtual environment ([#&#8203;7471](python-poetry/poetry#7471)).
-   Fix an issue where poetry crashed with a `JSONDecodeError` when running a Python script that produced certain warnings ([#&#8203;6665](python-poetry/poetry#6665)).

##### Docs

-   Add advice on how to maintain a poetry plugin ([#&#8203;6977](python-poetry/poetry#6977)).
-   Update tox examples to comply with the latest tox release ([#&#8203;7341](python-poetry/poetry#7341)).
-   Mention that the `poetry export` can export `constraints.txt` files ([#&#8203;7383](python-poetry/poetry#7383)).
-   Add clarifications for moving configuration files ([#&#8203;6864](python-poetry/poetry#6864)).
-   Mention the different types of exact version specifications ([#&#8203;7503](python-poetry/poetry#7503)).

##### poetry-core ([`1.5.1`](https://github.com/python-poetry/poetry-core/releases/tag/1.5.1))

-   Improve marker handling ([#&#8203;528](python-poetry/poetry-core#528),
    [#&#8203;534](python-poetry/poetry-core#534),
    [#&#8203;530](python-poetry/poetry-core#530),
    [#&#8203;546](python-poetry/poetry-core#546),
    [#&#8203;547](python-poetry/poetry-core#547)).
-   Validate whether dependencies referenced in `extras` are defined in the main dependency group ([#&#8203;542](python-poetry/poetry-core#542)).
-   Poetry no longer generates a `setup.py` file in sdists by default ([#&#8203;318](python-poetry/poetry-core#318)).
-   Fix an issue where trailing newlines were allowed in `tool.poetry.description` ([#&#8203;505](python-poetry/poetry-core#505)).
-   Fix an issue where the name of the data folder in wheels was not normalized ([#&#8203;532](python-poetry/poetry-core#532)).
-   Fix an issue where the order of entries in the RECORD file was not deterministic ([#&#8203;545](python-poetry/poetry-core#545)).
-   Fix an issue where zero padding was not correctly handled in version comparisons ([#&#8203;540](python-poetry/poetry-core#540)).
-   Fix an issue where sdist builds did not support multiple READMEs ([#&#8203;486](python-poetry/poetry-core#486)).

##### poetry-plugin-export ([`^1.3.0`](https://github.com/python-poetry/poetry-plugin-export/releases/tag/1.3.0))

-   Fix an issue where the export failed if there was a circular dependency on the root package ([#&#8203;118](python-poetry/poetry-plugin-export#118)).

</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:eyJjcmVhdGVkSW5WZXIiOiIzNC4xNTIuNSIsInVwZGF0ZWRJblZlciI6IjM0LjE1Mi41In0=-->

Reviewed-on: https://git.walbeck.it/walbeck-it/docker-python-poetry/pulls/655
Co-authored-by: renovate-bot <bot@walbeck.it>
Co-committed-by: renovate-bot <bot@walbeck.it>
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.

2 participants