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鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

Replace pep425tags w/ packaging.tags #6908

Closed
brettcannon opened this issue Aug 22, 2019 · 16 comments 路 Fixed by #7354
Closed

Replace pep425tags w/ packaging.tags #6908

brettcannon opened this issue Aug 22, 2019 · 16 comments 路 Fixed by #7354

Comments

@brettcannon
Copy link

@brettcannon brettcannon commented Aug 22, 2019

It's the entire reason packaging.tags was written to support Python 2.7. 馃槃 Plus packaging.tags has a much more thorough list of potential tags than pep425tags has, so it should lead to more wheels being flagged as supported (especially for PyPy and other alternative VMs).

Link to packaging.tags: https://github.com/pypa/packaging/blob/master/packaging/tags.py

@triage-new-issues triage-new-issues bot added the triage label Aug 22, 2019
@cjerdonek

This comment has been minimized.

Copy link
Member

@cjerdonek cjerdonek commented Aug 22, 2019

Are there any plans to add type annotations to packaging.tags? pep425tags is one of the modules in pip that is fully annotated, so it鈥檇 be nice to at least stay the same on that front..

@cjerdonek

This comment has been minimized.

Copy link
Member

@cjerdonek cjerdonek commented Aug 22, 2019

Also, do you recall the date / commit hash when packaging.tags was forked from pep425tags? That would let people review the changes to pep425tags since then to make it easier to see that feature parity is being preserved.

@pradyunsg

This comment has been minimized.

Copy link
Member

@pradyunsg pradyunsg commented Aug 23, 2019

Another thing that's needed is being able to generate tags for a different platform/version/implementation/abi, IIUC.

    :param versions: a list of string versions, of the form ["33", "32"],
        or None. The first version will be assumed to support our ABI.
    :param platform: specify the exact platform you want valid
        tags for, or None. If None, use the local system platform.
    :param impl: specify the exact implementation you want valid
        tags for, or None. If None, use the local interpreter impl.
    :param abi: specify the exact abi you want valid
        tags for, or None. If None, use the local interpreter abi.

And noarch, for only generating "none" tags.

@di

This comment has been minimized.

Copy link
Member

@di di commented Aug 23, 2019

We also probably want a new release of packaging before we can do this (to avoid vendoring attrs in).

@brainwane brainwane mentioned this issue Aug 23, 2019
18 of 25 tasks complete
@brettcannon

This comment has been minimized.

Copy link
Author

@brettcannon brettcannon commented Aug 23, 2019

@cjerdonek no specific plans for type annotations, but I personally have no issue adding them. Probably a broader question of whether 'packaging' itself wants to go down the route of adding type annotations.

As for when it was forked, it was never actually forked. 馃槃 I wrote packaging.tags from scratch and tried to match the semantics of pep425tags as reasonable but also expand them where I thought it was falling short. But if you want a rough date, probably Dec 14, 2018 when I created the PR. But I did check earlier this week whether there were tag differences at least on Windows and I fixed the one critical case.

@pradyunsg expanding the API to allow for that wouldn't be hard. First pass on the module specifically kept the API small. If you want me to start thinking about how to let sys_tags() or some new function take specific details then I can. Just open an issue on 'packaging' and assign it to me with the exact details of what you need to allow for specifying (e.g. do you explicitly need separate versions or separate interpreter versions, IOW do you want to specify by wheel tag or lower-level breakdown?).

@di I assumed a release would be made when pip was ready to roll up everything that has gone in at this point. For me the key point of doing this issue was to make sure folks like @cjerdonek were aware that packaging.tags existed so if something happened to pep425tags it could also get flagged down to packaging.tags.

@cjerdonek

This comment has been minimized.

Copy link
Member

@cjerdonek cjerdonek commented Aug 24, 2019

Probably a broader question of whether 'packaging' itself wants to go down the route of adding type annotations.

@brettcannon I think that would be a good idea. I don't think all of packaging would need to be annotated before incorporating packaging.tags, but I think at least annotating tags before then would be good.

Also, in terms of behavior and features, are there any differences with pep425tags that you're aware of (aside from pep425tags being incomplete)? Have you checked? I know, for example, that there's a case where pep425tags will log debug messages:

val = get_config_var(var)
if val is None:
if warn:
logger.debug("Config variable '%s' is unset, Python ABI tag may "
"be incorrect", var)
return fallback()

but packaging.tags doesn't seem to do any logging.

PS - I added a link to packaging.tag's source file in your original comment.

@pradyunsg

This comment has been minimized.

Copy link
Member

@pradyunsg pradyunsg commented Aug 25, 2019

open an issue on 'packaging' and assign it to me with the exact details of what you need to allow for specifying

Done! Here it is: pypa/packaging#187

@pradyunsg

This comment has been minimized.

Copy link
Member

@pradyunsg pradyunsg commented Aug 25, 2019

We also probably want a new release of packaging before we can do this (to avoid vendoring attrs in).

I don't think this'd be finished before the next release cycle of pip TBH. So, the lack of a packaging release isn't a major concern to me. I'm pretty sure we'll get to it before this. ^>^

@di

This comment has been minimized.

Copy link
Member

@di di commented Aug 25, 2019

I don't think this'd be finished before the next release cycle of pip

Now that pypa/packaging#186 is merged, I am actually hoping to do this before the next release cycle, so we can get manylinux2014 compatibility into the earliest release possible.

@cjerdonek

This comment has been minimized.

Copy link
Member

@cjerdonek cjerdonek commented Aug 25, 2019

I also created an issue for adding type annotations to packaging.tags: pypa/packaging#189

@pradyunsg

This comment has been minimized.

Copy link
Member

@pradyunsg pradyunsg commented Aug 26, 2019

I am actually hoping to do this before the next release cycle

I'm not opposed - I just don't know if/when folks would be putting in the time to work on this, since as you can see, there's more details to figure out, for making this replacement of pep425tags. :)

@brettcannon

This comment has been minimized.

Copy link
Author

@brettcannon brettcannon commented Aug 26, 2019

@cjerdonek other than the logging, obviously the API, and the differences in what tags are emitted, I'm not explicitly aware of any other differences.

@cjerdonek

This comment has been minimized.

Copy link
Member

@cjerdonek cjerdonek commented Aug 26, 2019

I also noticed this recently, which is re: an enhancement that went into pip鈥檚 version: pypa/packaging#171

@di

This comment has been minimized.

Copy link
Member

@di di commented Nov 5, 2019

I think the last thing blocking this issue is pypa/packaging#221 (and the necessary work in pip itself).

@brettcannon

This comment has been minimized.

Copy link
Author

@brettcannon brettcannon commented Nov 6, 2019

Isn't pypa/packaging#187 also a blocker?

@di

This comment has been minimized.

Copy link
Member

@di di commented Nov 6, 2019

Apologies, forgot about that. Yes.

@lock lock bot added the S: auto-locked label Feb 8, 2020
@lock lock bot locked as resolved and limited conversation to collaborators Feb 8, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Linked pull requests

Successfully merging a pull request may close this issue.

4 participants
You can鈥檛 perform that action at this time.