No way to do operations on marker

[gaborbernat]

Currently, the marker has its data field private, so there's no way for someone to search for markers once parsed a requirement. My goal here is to filter for some extra, roughly:

extras=[r.value for l, _, r in (i for i in a.marker._markers if isinstance(i, tuple) and len(i) == 3) if l.value =='extra']

I propose to either:

• make ._markers public
• implement __iter__ that yields entries from _markers

I've needed this also in tox rewrite - https://github.com/tox-dev/tox/blob/rewrite/src/tox/tox_env/python/virtual_env/package/api.py#L186-L211

[uranusjr]

Could you describe what kind of operations you intend to perform? i.e. the operands and what result you want to get. I’m mostly just curious because my previous attempts on modifying the markers all ended in frustrations and I ended up giving up and just doing things like f"({marker1}) and ({marker2})".

[gaborbernat]

You can see in the link above I've provided; mostly just extracting and removing the extra marker 😊 By the way for me the extras property is empty, and they show up in the markers instead 🤔

[pradyunsg]

By the way for me the extras property is empty, and they show up in the markers instead 🤔

This is likely an understanding gap. The extras property maps to the content between the square brackets and is completely independent of how markers are processed.

name[foo]>=2,<3; extra == "bar"

req.extras is a set containing foo. The marker is a different part of the requirement, and is stored in req.marker. The "extra" property in the marker is parsed and stored as part of "things to evaluate when looking at this marker", and is unrelated to "extras of this package to install".

[uranusjr]

mostly just extracting and removing the extra marker

That makes sense, thanks. +1 to the __iter__ method from me, that sounds like a reasonable and useful addition to inspect a marker. We’ll need to find ways to deal with nested expressions but that should be technically not difficult.

The "extra" property in the marker is parsed and stored as part of "things to evaluate when looking at this marker", and is unrelated to "extras of this package to install".

I just want to add this has always been a great source of confusion to a lot of people and is one of the main reasons I hate extras.

[pradyunsg]

Yea, I think we can all agree that extras are a very messy thing. :)

[gaborbernat]

For being able to remove elements would be helpful to just make _markers public 🤔 rather than go down the __iter__ path

[pradyunsg]

+1 to the __iter__ method from me, that sounds like a reasonable and useful addition to inspect a marker. We’ll need to find ways to deal with nested expressions but that should be technically not difficult.

Hmm... I'm not sure what __iter__ would return. The only thing that I can think of is tokens of the expressions -- (1) that information is not stored currently and (2) I'm not sure that's super useful anyway?

For the kinds of things that @gaborbernat is describing, an expression AST seems more appropriate. That's basically how we store things currently too, handling nesting fairly organically. For exposing that as a public API, I think we basically need to figure out what all we need to expose (I wanna start small and strict), what modifying an AST looks like and how "gimme a marker from this AST" would work.

[gaborbernat]

Well my MVP functionality would be:

• Find all markers of type and value X
• Remove marker of type and value X (note removing a marker is not just remove the marker itself but also any operator it might be attached to).

[uranusjr]

Hmm... I'm not sure what __iter__ would return. The only thing that I can think of is tokens of the expressions -- (1) that information is not stored currently and (2) I'm not sure that's super useful anyway?

I’m thinking something like

>>> marker = Marker("(os_name == 'nt' or sys_platform == 'linux') and python_version <= '3.8'")
>>> submarkers = list(iter(marker))
>>> submarkers
[Marker("os_name == 'nt' or sys_platform == 'linux'"), "and", Marker("python_version <= '3.8'")]
>>> list(iter(submarkers[0]))
[Marker("os_name == 'nt'), "or", Marker("sys_platform == 'linux'")]
>>> list(iter(submarkers[1]))
[Marker("python_version <= '3.8'")]

So yeah, basically an AST, and maybe we can implement a more sophisticated API than just __iter__.

I don’t like exposing Marker._markers directly since pyparsing constructs should be an implementation detail. We should wrap everything exposed publicly in Marker (and maybe add a new type MarkerOperator or something).

[gaborbernat]

@uranusjr I'm ok with that 👍 As long as we implement del too, so users can remove markers 😊

[uranusjr]

Personally I prefer having Marker immutable instead and provide something like

>>> Marker.join([Marker("os_name == 'nt'"), "or", Marker("sys_platform == 'linux'")])
Marker("os_name == 'nt' or sys_platform == 'linux'")

or perhaps

>>> Marker("os_name == 'nt'") | Marker("sys_platform == 'linux'")
Marker("os_name == 'nt' or sys_platform == 'linux'")

[FFY00]

That would be useful. It would allow us replace usages such as this

requirement = packaging.requirements.Requirement(requirement_string)
if requirement.marker:  # append our extra to the marker
    requirement.marker = packaging.markers.Marker(
        str(requirement.marker) + f' and extra == "{extra}"'
    )
else:  # add our extra marker
    requirement.marker = packaging.markers.Marker(f'extra == "{extra}"')

It could turn into

requirement = packaging.requirements.Requirement(requirement_string)
extra_marker = packaging.markers.Marker(f'extra == "{extra}"')
if requirement.marker:
	requirement.marker += extra_marker
else:
	requirement.marker = extra_marker

Perhaps we could allow empty markers, and do

requirement = packaging.requirements.Requirement(requirement_string)
requirement.marker += packaging.markers.Marker(f'extra == "{extra}"')

(this is code I use when mapping PEP 621 optional-dependencies entries to core metadata)