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’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Adds output that reflects a package's supported Python interpreters #80

Closed
wants to merge 3 commits into from

Conversation

funkyfuture
Copy link
Contributor

i think that addresses proposal #43 by @RonnyPfannschmidt that i liked.

let me know what you think should still be addressed.

cat */PKG-INFO | python -c '
import json, re, sys
match_classifier = re.compile(
r"\s*Classifier: Programming Language :: Python :: (\d+\.\d+)$"

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

a correct implementation needs to consider requires_python and the list of supported python versions of githubs setup_python

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

i could argue that this component shouldn't serve for deferred metadata consistency validation and generally as well as particularly regarding the second concerns: the targeted application environment is the one where we want things to crash in order to fix upon. the provided outputs are supposed to be building blocks, afaik jq is available in the runner environment.

Copy link

@RonnyPfannschmidt RonnyPfannschmidt Dec 27, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The reason the version list should come from requires-python is that that list Will be what pip will consider/respect

The trove classifiers are neither complete nor under mandatory consideration

Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, requires-python is the canonical metadata we ought to check. It would be a nice-to-have to raise an error, if it it contradicts the classifiers, but the classifiers are just a FYI.

That said there's the big question of how would we decide the upped Python version bound?

You get that from the classifiers which kinda made them an indicator of what the CI is checking against. But the supported upper bound would have to technically change whenever GitHub Actions add a new Python version?

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If no upper bound was given,then it's a good idea to get a failure as soon as a new python version gets added that fails

I'd consider known good upper bounds as out of scope for this one

@funkyfuture
Copy link
Contributor Author

frohes neues! as your requirements are different than mine, i'm closing this.

@funkyfuture funkyfuture closed this Jan 2, 2024
@hynek
Copy link
Owner

hynek commented Jan 2, 2024

frohes neues! as your requirements are different than mine, i'm closing this.

Honestly, I don't think they are.

There seems to be two things at play that sounds the same but aren't:

  1. The explicit enumeration of supported Python versions.
  2. requires-python providing packaging metadata

To me, it would make the most sense to use 1. to drive the CI and – and this doesn't have to be the same PR/step – checking if 1 and 2 are contradicting each other.

Since it's possible to define quite intricate versions in 2, I'm not even sure how realistic that is. I remember e.g. most asyncio packages requiring a later 3.5 minor release due to asyncio bugs.

@funkyfuture
Copy link
Contributor Author

Honestly, I don't think they are.

honestly, i'm unsure whether i followed correctly until now.

There seems to be two things at play that sounds the same but aren't:

my impression is that the difference was clear all the time.

The explicit enumeration of supported Python versions.

i assume that you mean the respective trove classifiers with that?

To me, it would make the most sense to use 1. to drive the CI

i agree, because I do assume that all packages are supposed to be built and shipped with complete and correct metadata. (otherwise i consider it a bug that requires a patch release.)

checking if 1 and 2 are contradicting each other.

here i attempted to express that this a valid (even important) concern, but also that this proposal here isn't trying to solve that. it should be part of a metadata validation unit.

Since it's possible to define quite intricate versions in 2, I'm not even sure how realistic that is.

my consideration still is that it isn't worth the effort, particularly weighed against the previous argument.


if the above reflects a consensus, feel free to re-open this pr.

@hynek
Copy link
Owner

hynek commented Jan 8, 2024

Currently, the only consensus that is needed is mine. We can work on this and merge it and let #43 open for further thinking/working on problem 2.

@hynek hynek reopened this Jan 8, 2024
@RonnyPfannschmidt
Copy link

As project fmt fixes the python version list, I consider it a good enough source

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants