feat(markers): support 'extras' and 'dependency_groups' markers by frostming · Pull Request #888 · pypa/packaging

frostming · 2025-04-03T01:24:58Z

Signed-off-by: Frost Ming [email protected]

Resolve #885

Signed-off-by: Frost Ming <[email protected]>

brettcannon · 2025-04-03T20:28:12Z

The PR LGTM (and thanks, @frostming !), but since @pradyunsg wrote most of the code being changed I want to give him a chance to do a review if he has the time. If not I will merge this in about a week.

pradyunsg · 2025-04-04T20:29:45Z

src/packaging/markers.py

        """
        current_environment = cast("dict[str, str]", default_environment())
-        current_environment["extra"] = ""
+        current_environment.update(extra="", extras=set(), dependency_groups=set())


We should not be setting these in the evaluation environments unconditionally, since that would allow packages to use the extras and dependency_groups marker variables in regular dependency declarations (outside of the context of a lockfile).

This would make it ~impossible for downstream tools to implement the following behaviour from PEP 751's specification update:

Outside of the context of lock files, these two variables should result in an error like all other unknown variables.

It would be good to add an explicit test for the unknown variable error case that this pertains to.

Outside of the context of lock files, these two variables should result in an error like all other unknown variables.

But other unknown markers are thrown as InvalidMarker errors during the parse phase. Should we, and can we keep consistent for extras? That may require a different version of parsing function, possibly with a flag

I made it a "should" statement for a reason. We could add a flag to evaluate() to specify the context and also clean things up for extra while we are at it, otherwise there's precedent of this code being sloppy/permissive in what it accepts.

@brettcannon thanks for the clarification, not sure if I interpret your feedback correctly

What I was thinking was instead of a for_lock argument, we have a context argument. That would take an enum that specified under what context the marker was being evaluated. We could have metadata which matches what it does now where extra is always set (and would be the default). We could have lock_file which doesn't have extra but has extras and dependency_groups always set. And then we could have requirement which has none of extra, extras, or dependency_groups automatically set.

FYI I'm not attached to any of these names.

@pradyunsg does the approach Frost took work for you (it does for me)?

noxfile.py

tests/test_markers.py

Signed-off-by: Frost Ming <[email protected]>

brettcannon

Just a docstring change and docs will need updating.

src/packaging/markers.py

Co-authored-by: Brett Cannon <[email protected]>

Signed-off-by: Frost Ming <[email protected]>

brettcannon

I think

packaging/docs/markers.rst

Lines 61 to 74 in e624d8e

    
           .. method:: evaluate(environment=None) 
        
           Evaluate the marker given the context of the current Python process. 
        
           :param dict environment: A dictionary containing keys and values to 
        
                                    override the detected environment. 
        
           :raises: UndefinedComparison: If the marker uses a comparison on strings 
        
                                         which are not valid versions per the 
        
                                         :ref:`specification of version specifiers 
        
                                         <pypug:version-specifiers>`. 
        
           :raises: UndefinedEnvironmentName: If the marker accesses a value that 
        
                                              isn't present inside of the environment 
        
                                              dictionary. 
        
           :rtype: bool

needs an update and then if @pradyunsg wants to say if he's good with the method signature change (I am). Otherwise LGTM!

brettcannon · 2025-04-15T20:50:33Z

FYI linting failed.

Signed-off-by: Frost Ming <[email protected]>

brettcannon · 2025-04-16T22:13:21Z

LGTM! We will give @pradyunsg until next week to weigh in, else I will merge this.

pradyunsg

Let's do this! Thanks @frostming!

This PR contains the following updates: | Package | Type | Update | Change | |---|---|---|---| | [packaging](https://github.com/pypa/packaging) | project.dependencies | major | `==24.2` -> `==25.0` | --- ### Release Notes <details> <summary>pypa/packaging (packaging)</summary> ### [`v25.0`](https://github.com/pypa/packaging/releases/tag/25.0) [Compare Source](pypa/packaging@24.2...25.0) #### What's Changed - Re-add a test for Unicode file name parsing by [@Siddhesh-Agarwal](https://github.com/Siddhesh-Agarwal) in pypa/packaging#863 - Upgrade to ruff 0.9.1 by [@DimitriPapadopoulos](https://github.com/DimitriPapadopoulos) in pypa/packaging#865 - Add support for PEP 738 Android tags by [@mhsmith](https://github.com/mhsmith) in pypa/packaging#880 - feat(markers): support 'extras' and 'dependency_groups' markers by [@frostming](https://github.com/frostming) in pypa/packaging#888 #### New Contributors - [@Siddhesh-Agarwal](https://github.com/Siddhesh-Agarwal) made their first contribution in pypa/packaging#863 - [@mhsmith](https://github.com/mhsmith) made their first contribution in pypa/packaging#880 - [@frostming](https://github.com/frostming) made their first contribution in pypa/packaging#888 **Full Changelog**: pypa/packaging@24.2...25.0 </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 is behind base branch, or you tick the rebase/retry checkbox. 🔕 **Ignore**: Close this PR and you won't be reminded about this update again. --- - [ ] If you want to rebase/retry this PR, check this box --- This PR has been generated by [Renovate Bot](https://github.com/renovatebot/renovate).  Reviewed-on: https://git.tainton.uk/repos/pypilot/pulls/329 Reviewed-by: Luke Tainton <[email protected]> Co-authored-by: Renovate [BOT] <[email protected]> Co-committed-by: Renovate [BOT] <[email protected]>

frostming added 2 commits April 3, 2025 09:24

feat(markers): support 'extras' and 'dependency_groups' markers

26cdf27

Signed-off-by: Frost Ming <[email protected]>

fix: types

200c455

Signed-off-by: Frost Ming <[email protected]>

frostming force-pushed the feat/pep751-markers branch from 5af59cc to 200c455 Compare April 3, 2025 01:49

frostming mentioned this pull request Apr 3, 2025

New standard: Support PEP 751 pdm-project/pdm#3439

Closed

fix: add tests

bf879e6

Signed-off-by: Frost Ming <[email protected]>

brettcannon self-requested a review April 3, 2025 20:24

brettcannon approved these changes Apr 3, 2025

View reviewed changes

brettcannon requested a review from pradyunsg April 3, 2025 20:26

pradyunsg reviewed Apr 4, 2025

View reviewed changes

pradyunsg added the packaging.markers label Apr 4, 2025

pradyunsg reviewed Apr 4, 2025

View reviewed changes

noxfile.py Outdated Show resolved Hide resolved

tests/test_markers.py Show resolved Hide resolved

frostming added 2 commits April 5, 2025 08:06

fix: changes for feedbacks

ceef363

Signed-off-by: Frost Ming <[email protected]>

fix: add for_lock flag to evaluate()

5be474f

Signed-off-by: Frost Ming <[email protected]>

frostming force-pushed the feat/pep751-markers branch 2 times, most recently from 8fbf371 to 28a9c05 Compare April 9, 2025 01:31

fix: use an enum instead of flag

6f37ebf

Signed-off-by: Frost Ming <[email protected]>

frostming force-pushed the feat/pep751-markers branch from 28a9c05 to 6f37ebf Compare April 9, 2025 02:00

brettcannon requested changes Apr 14, 2025

View reviewed changes

src/packaging/markers.py Outdated Show resolved Hide resolved

src/packaging/markers.py Outdated Show resolved Hide resolved

frostming and others added 2 commits April 15, 2025 08:40

Apply suggestions from code review

bed6db8

Co-authored-by: Brett Cannon <[email protected]>

chore: add changelog

7a0a7d2

Signed-off-by: Frost Ming <[email protected]>

brettcannon requested changes Apr 15, 2025

View reviewed changes

frostming and others added 2 commits April 16, 2025 08:26

fix: lint error

941c61c

Signed-off-by: Frost Ming <[email protected]>

Merge branch 'main' into feat/pep751-markers

a132f85

brettcannon self-requested a review April 16, 2025 17:48

brettcannon approved these changes Apr 16, 2025

View reviewed changes

pradyunsg approved these changes Apr 17, 2025

View reviewed changes

brettcannon merged commit 3910129 into pypa:main Apr 17, 2025
33 checks passed

brettcannon mentioned this pull request Apr 17, 2025

Release for pip 25.1 #890

Closed

never147 mentioned this pull request Apr 22, 2025

Bump packaging from 24.2 to 25.0 never147/ansible-desktop#40

Merged

renovate-bot mentioned this pull request Jun 12, 2025

chore(deps): update all dependencies googleapis/python-datastore#566

Closed

1 task

h4l mentioned this pull request Jul 26, 2025

Fix Marker non-version-to-version comparison throwing InvalidVersion #883

Closed

	.. method:: evaluate(environment=None)

	Evaluate the marker given the context of the current Python process.

	:param dict environment: A dictionary containing keys and values to
	override the detected environment.
	:raises: UndefinedComparison: If the marker uses a comparison on strings
	which are not valid versions per the
	:ref:`specification of version specifiers
	<pypug:version-specifiers>`.
	:raises: UndefinedEnvironmentName: If the marker accesses a value that
	isn't present inside of the environment
	dictionary.
	:rtype: bool

Conversation

frostming commented Apr 3, 2025

Uh oh!

brettcannon commented Apr 3, 2025

Uh oh!

pradyunsg Apr 4, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

pradyunsg Apr 4, 2025

Choose a reason for hiding this comment

Uh oh!

pradyunsg Apr 4, 2025

Choose a reason for hiding this comment

Uh oh!

frostming Apr 5, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

brettcannon Apr 7, 2025

Choose a reason for hiding this comment

Uh oh!

frostming Apr 8, 2025

Choose a reason for hiding this comment

Uh oh!

brettcannon Apr 8, 2025

Choose a reason for hiding this comment

Uh oh!

brettcannon Apr 14, 2025

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

brettcannon left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

brettcannon left a comment

Choose a reason for hiding this comment

Uh oh!

brettcannon commented Apr 15, 2025

Uh oh!

brettcannon commented Apr 16, 2025

Uh oh!

pradyunsg left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

pradyunsg Apr 4, 2025 •

edited

Loading

frostming Apr 5, 2025 •

edited

Loading