-
-
Notifications
You must be signed in to change notification settings - Fork 5.5k
-
-
Notifications
You must be signed in to change notification settings - Fork 5.5k
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
PyPI: Python version from classifiers vs. requires_python #5551
Comments
Note: If someone could explain to me how python-version is parsed here, I could try doing it myself. For example, I might want to know where |
So you're saying instead of
|
Yeah exactly. That sounds like a good choice of URL to me 👍
I'm guessing from the strike through you already figured it out, but I just used a static badge to demonstrate. The URL for that would be https://img.shields.io/badge/python-%3E%3D3.4%2C%3C4.0-blue |
Great, I'll do some browsing and see if I can make a PR! Or not, to which I'll let you know.
Um.... no? lol I used the strike because it was technically offtopic.... 🤷♂️... I should've known though... |
See #5576 for the PR related |
Reopening as referenced PR hasn't been merged and seems additional discussion is needed |
@paulmelnikow You're saying to merge the classifiers and pyrequires? Which one would we parse first? Do we show both? How would this work? |
pip only looks at
https://packaging.python.org/guides/distributing-packages-using-setuptools/#python-requires
My thinking on recommending that we just make this a separate badge is that the format of
In order to take into account both in one badge we've either got to try and convert ranges into a list of versions to retain the current format (knowing how to turn Given that I'm in favour of just saying PyPI - Python Version (from python_requires): are two different badges. My second-preference option would be to change |
Oh, interesting! PyPI clearly does read both. An example project that shows both is requests, and one that shows just the classifier is polliwog. I did not know pip uses I don't see a good reason to have two different badges for showing the the Python versions supported by a library. I'm not sure why authors would prefer the
I guess this is my first choice, mainly because I don't think we should have two different badges for the same piece of information …
… and I'm fine with this if you think it's important. |
I have already created a PR regarding two different badges. Seeing as making a combined badge versus two separate badges is still in discussion, I'll hold off the PR for now until we decide which option to go with. My opinion would be to make two different badges, as it's easier for me to make. I'm not experienced with js, as I've said before, and making two different badges has already proven to be easier for me. However, making them one badge and including |
Makes sense, let's sort out the discussion first. If you can't finish the PR, it's okay, maybe someone else will want to work on this, or maybe I can pick it up. Hmm, the Requires: The version from classifiers shows 2.7 | 3.5 | 3.6 | 3.7 | 3.8 which is much easier to understand.
Now I understand now what you meant here. I guess I'm still coming from a different place here, which is I think we should try to have one badge, and make the default behavior friendly and correct, even though it's a change from the past behavior. |
The documentation says that using We could look at this by iterating through every version (major, minor, and micro) and delete the ones that aren't allowed. Minor and Micro could be replaced by * if all remain. However, this wouldn't have any need for |
Yeah in a lot of cases is much clearer than Fundamentally, The other reason I think its important to provide a choice is because this wouldn't be a breaking change in the sense that https://img.shields.io/pypi/pyversions/django.svg renders a badge before and https://img.shields.io/pypi/pyversions/django.svg will render a badge after. But changing which value from the API response we display on the badge and the visual format of the badge (in lots of cases) is a big behavioural change. For a popular badge like this, if we make a big change like that to the default behaviour there will be some non-zero number of users who will want it to continue to work the way it has worked for the last ~5 years and I think we should provide a way to do that even if we changed the default, especially given we can (its not like classifiers are being removed or anything). |
I agree that it would change the way it looks and many people could become confused and may want the original back. However, since most python_requires uses only one "requires" it would be fine to use that as default. But if there is more than one "requires" we could use classifiers instead. Of course, you can always change default with |
I still feel like wrangling these formats is sort of an implementation detail that makes our life difficult, rather than something we fundamentally want to provide two options for.
I agree, it doesn't look trivial. Although I think this is what I'd like to try to do: attempt to provide a nice human-readable badge by interpreting Maybe the first course of action is to attempt to produce that function, see how good the results are, and then we can see if there is a use case for the old behavior (i.e. a project where we can't produce the right thing from Normally I don't advocate deep logic like this in Shields, though the state of Python packaging being what it is – and in this case specifically, two ways of specifying the same thing! – I think it's worth our effort to paper over the complexity. There are a few other precedents, such as the PHP version badges which fetch the latest release of PHP. I could also put the function in its own npm module. Added: There is a JS implementation of PEP 440 which could help. |
This is a point of concern for me, especially in light of other badge changes being applied semi-recently and the feedback we subsequently received from users. If a change could potentially result in a user seeing one badge message Regardless of our desired state, the status quo bias weighs heavily. If we're targeting making such a breaking change, and do so in a permanent way (i.e. no back to old (also haven't been following super closely so apologies if I've misunderstood something 😄) |
Agree with requesting comments before making the change. |
So we're going to poll the public IF we decide on combining the two badges? That's sensible. What would happen if the public goes against combining? Should we make the default classifiers? Make two separate badges? Etc... |
I propose we use the list of Active Python Releases at https://www.python.org/downloads/, and filter that using For packages with classifiers, this means we'll see the same thing as before. For packages with One implementation challenge is getting a machine-readable list of active python releases. We could use the ftp listing, though it shows prereleases so we'd have to filter those out. I'd suggest that initially I hard-code the list from that web page, since it only changes every couple of years and it's minimal maintenance burden. Then I can open a follow-up issue to reach out to the python.org folks and see if they have a suggestion about fetching a machine-readable list. |
This is a good indication that different users are going to have different expectations of how this should work: It is not completely clear-cut how this should work and will end up being somewhat opaque to users.
I think if you want the badge to look at a different value, the solution should be to change the badge(params) not change your package manifest. Broadly although #5576 is not currently mergeable I think I'm still in favour of that general approach over #5592 |
I'm trying to fix #5576 's problem so if someone can help, please do. It's very confusing.
Should we poll this right now? |
I recognize that I'm a bit in the weeds on this, so please bear with me…
This doesn’t really make sense in the context of my proposal. Let me try to clarify what I meant.
If the PyPI status, implementation, python, or django version badges displayed the wrong information today, it means the project has been published with the wrong information. You’d fix them by changing the classifiers and republishing, not changing the badge. The same is true of of the npmjs dependency, Node version, and
Since classifiers would take priority over As far as I can recall, when we have multiple badges that display the same piece of information, they are pulled from completely different data sources. An example is the Node version pulled from npmjs vs the Node version pulled from a package.json file on GitHub. The Python version badges are for users to know what Python versions a package supports, and I feel pretty strongly that it's one badge since the source is PyPI. While it's unfortunate that PyPI provides two ways of expressing this information, #5550 was a request to consider the source where it was specified instead of showing "unknown". It wasn't a request to choose between two conflicting sources. Choosing the source feels less like a different badge, and more like the I'm open to making a parameter here too. Though before we do, I think it would be useful to find a concrete example that justifies it (or wait for someone to provide one). So far I'm not able to think of such an example. These are the things I'm thinking about:
Maybe, though I think it's more a result of implementing it the other way, and then not being able to identify a use case where a badge should use requires_python when classifiers are present. |
requires_python was meant to force pip from not installing if the python version was incorrect. I believe that would be the right source of data, especially because classifiers don't have a micro option. Specificity is always the best way to go in my mind.
However, you are also right. It makes no sense for someone to write two different sets of versions for their package. However if this were to happen, I would go towards using the more readable option. In some cases this could be requires, but in all other cases, it would probably be classifiers. |
I think part of the reason we disagree is because we're thinking about slightly different things. I'm partly thinking about this also giving us an alternate display format. So to give a concrete example from one of my projects where I actually use this badge https://github.com/chris48s/arcgis2geojson , one of the things that is potentially quite nice about being able to pick which value you want is if your list of classifiers is long but your - https://img.shields.io/pypi/pyversions/arcgis2geojson.svg is quite long, especially once 3.9 gets added to that list. It would be quite nice to be able to switch to a badge that does the equivilent of Note my classifiers list is not inconsistent with my
Interesting philosophical question in its own right. If a tree falls in the forest.. 🌳
that's just silly. I don't think PyPI actively stops you doing it (I've not tried), but it doesn't mean anything sensible. I think there are probably 2 sensible-ish cases where The common one is going to be where you've set your
At the point you uploaded the package, they were totally consistent but then python 3.9 comes out, I guess maybe another valid-ish case where
so I'm not going to actively prevent you installing on python 3.4 and it will probably work, but the classifiers describe my test matrix or what I'm willing to deal with support requests on (e.g: because my CI provider makes it hard to test on unsupported versions, or whatever but I'm not actually relying on any feature/syntax that is only available in python 3.5+). Not sure how valid/common that is - I don't have a concrete example. I don't think that tells us anything other what what we already know from https://packaging.python.org/guides/distributing-packages-using-setuptools/#classifiers though: Hopefully we can arrive at a consensus on this one way or another fairly soon. I appreciate that although we've got different perspectives at the moment we're keeping the conversation constructive (I think) and we both want to arrive at a solution which is correct and provides value to the community. Simultaneously, there is something a bit exhausting about spending the evening drafting another wall o' text on this. 😄 Might need to take a break |
Yea, I just wanna echo that. I feel like we started in really different places and covered a lot of ground, which feels constructive to me. It feels good to be engaging more deeply and passionately on Shields, and kinda neat that this is a topic we both have strong feelings about. This has felt constructive to me, too. Also spent a while on this last night and felt a little exhausted after. Shall we let it rest a few days? |
I guess we could let this go idle. If anyone has any ideas they could just send it here. But we can finally come back "officially" with a fresh mind later. |
Python packaging is a topic capable of driving the most rational minds to the edge of reason at times. |
Too true. |
No pressure to jump back into this conversation yet, but I've been slowly churning this over and I've thought of another interesting example of a case where This package can be installed on python 3.5+ (it is compatible - https://github.com/python/typing/blob/95d6776c0d07e7b096ca153ed6c5b1b30db738e1/setup.py#L70 but the classifiers declare support for (or perhaps "relevance to") python 2.7 and python 3.4 (which is where this package will actually do something ) Admittedly this is a quite uncommon type of package. Simultaneously I reckon I'm happy to say this list of authors know how to do python packaging properly: https://github.com/python/typing/blob/95d6776c0d07e7b096ca153ed6c5b1b30db738e1/setup.py#L61 |
I think you meant 2.7 and 3.4? I'm amused that you found a package that's relevant to certain versions of Python and compatible with others. 😀 In thinking about what badge or badges we want to have available for this package…
|
Yes sorry - it backports types to python 3.4 - they're native in 3.5
Agreed. The list of classifiers (whatever the package maintainer has decided that means) on a badge is a useful thing. That's what we already have.
I think it is also useful to be able to communicate compatibility as a different concept from classifiers sometimes.
Yes. If we are going to turn the range into a list, we have to take into account all versions matching the constraint. Assuming the maintainer only cares about active versions is too much of a leap - we don't know. i.e: if the maintainer says the constraint is |
Hmm, I think this is enough of a break? I still have no idea how to do this, still very inexperienced with javascript. Current ideas we had:
Should we continue with one idea? Or should we still discuss more ideas and find the best fit. |
Yeah its a useful bump 👍 I think I'm still in favour of 2 badges: one which shows Since having this conversation I just swapped out all the badges on all my own python projects to a custom json badge that shows |
It is also worth noting that there is a slightly different but somewhat related conversation going on in #6013 |
Hmm, seeing as there haven't been many updates on this Issue lately, I think I will close this. I think the best way is what Chris just showed with the link/embed above. |
In #5550 I noted that to get pyversion, you need to use classifiers. However, there are a lot of python libraries out there that also use
python_requires
instead.Instead of making a feature proposition there, I decided to make a new issue that was directed at this task instead of using an existing 'question' issue.
I'm proposing a feature where the pyversions look for python_requires before parsing classifiers.
The text was updated successfully, but these errors were encountered: