-
Notifications
You must be signed in to change notification settings - Fork 31
Project metadata: Remove setup.py. Improve Python compatibility.
#759
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
base: main
Are you sure you want to change the base?
Conversation
This comment was marked as spam.
This comment was marked as spam.
1feba49 to
9daed8a
Compare
setup.pysetup.py. Improve Python compatibility.
4b4e060 to
fd534ff
Compare
| "Operating System :: OS Independent", | ||
| "Programming Language :: Python", | ||
| "Programming Language :: Python :: 3", | ||
| "Programming Language :: Python :: 3.10", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Shouldn't we also add the other supported python versions here?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks. Fixed.
seut
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good to me, thanks!
Pinging @mfussenegger to ensure we do not miss any feedback on this.
| description = "CrateDB Python Client" | ||
| authors = [{ name = "Crate.io", email = "[email protected]" }] | ||
| requires-python = ">=3.10" | ||
| requires-python = ">=3.6" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why do we lower this now? Afaik newer versions intentionally bumped this so that pip install on older python versions would install an old crate-python version - allowing us to drop compat logic for older versions.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
setup.py says python_requires=">=3.6", I've just transferred this to pyproject.toml.
| # check if tag to create has already been created | ||
| WORKING_DIR=`dirname $0` | ||
| VERSION=`python setup.py --version` | ||
| VERSION=`hatch project metadata version` |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This implies that there's now a hatch executable available in $PATH. Looks like this isn't mentioned in DEVELOP.rst - the whole file looks sort of outdated, together with the bootstrap.sh
Can we simplify that, so there's some sort of standard uv venv venv; uv pip install -Ue ".[dev]" with no special custom logic?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I've created a dedicated ticket. Thank you.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why not fix it? Seems like things are sort of broken as they are in this PR
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Seems like things are sort of broken as they are in this PR.
This patch was merely thought cosmetic to remove setup.py as intended. GH-739 already made the project use Hatch, because pyproject.toml takes precedence even when using the traditional setuptools build backend like before. In this spirit, the project would already be broken right now?
Why not fix it?
Switching to setuptools again requires more changes, because standard setuptools uses a different format in pyproject.toml than hatch. Let's discuss here?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Switching to setuptools again requires more changes, because standard setuptools uses a different format in pyproject.toml than hatch. Let's discuss #766 (comment)?
I don't see the advantage of going from A (broken) -> B (this version - also broken) -> C (version that works) when we can do A (broken) -> C (not broken).
Also note that this doesn't require switching away from hatch. It just needs to become a build time dependency that's documented. Could even make use of uvx hatch if we already depend on uv (which should also be documented if that's the case, and isn't mentioned)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hi. Apologies that I still can't follow what is exactly broken here. Can you share an error message? Hatch is currently a build time dependency, and this works well.
[build-system]
requires = ["hatchling", "versioningit"]
build-backend = "hatchling.build"There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I might be seeing the problem now: Is it hatchling vs. hatch? So, let's just add hatch, right! See 8a9a172.
Just maintenance while modernizing project metadata: Remove outmoded
setup.py, and transfer a few more details to newpyproject.toml.