packaging icon indicating copy to clipboard operation
packaging copied to clipboard

No way to do operations on marker

Open gaborbernat opened this issue 3 years ago • 16 comments

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

gaborbernat avatar Jul 16 '21 07:07 gaborbernat

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})".

uranusjr avatar Jul 21 '21 07:07 uranusjr

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

gaborbernat avatar Jul 21 '21 07:07 gaborbernat

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".

pradyunsg avatar Jul 21 '21 07:07 pradyunsg

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.

uranusjr avatar Jul 21 '21 08:07 uranusjr

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

pradyunsg avatar Jul 21 '21 10:07 pradyunsg

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

gaborbernat avatar Jul 21 '21 10:07 gaborbernat

+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.

pradyunsg avatar Jul 21 '21 10:07 pradyunsg

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).

gaborbernat avatar Jul 21 '21 10:07 gaborbernat

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).

uranusjr avatar Jul 21 '21 12:07 uranusjr

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

gaborbernat avatar Jul 21 '21 13:07 gaborbernat

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'")

uranusjr avatar Jul 21 '21 15:07 uranusjr

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)

FFY00 avatar Sep 08 '21 16:09 FFY00

Actually, we could take a cue from sets and provide overrides for & and | (which then includes &= and |=).

brettcannon avatar Sep 08 '21 23:09 brettcannon

Arrived here from https://discuss.python.org/t/programmatically-getting-non-optional-requirements-of-current-directory/26963/5?u=astrojuanlu. If I understand correctly, this would enable users to filter out requirements that have an extras marker, am I right? If not, should I open a different issue?

@layday was ~~advocating~~ suggesting "addding a partial_match flag to evaluate", I'm not 100 % sure if it would cover the same use case.

astrojuanlu avatar May 22 '23 17:05 astrojuanlu

advocating

Merely suggested ;P

layday avatar May 22 '23 18:05 layday

If I understand correctly, this would enable users to filter out requirements that have an extras marker, am I right?

Yes; the idea is the markers on a requirements object would be directly exposed so you can introspect them.

brettcannon avatar May 24 '23 18:05 brettcannon