Conversation
This comment was marked as spam.
This comment was marked as spam.
amotl
force-pushed
the
rm-setup-py
branch
4 times, most recently
from
1feba49 to
9daed8a
Compare
amotl
changed the title
setup.pysetup.py. Improve Python compatibility.
amotl
requested review from
seut and
surister
and removed request for
surister
amotl
force-pushed
the
rm-setup-py
branch
2 times, most recently
from
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.
Shouldn't we also add the other supported python versions here?
seut
left a comment
seut
left a comment
There was a problem hiding this comment.
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 = "office@crate.io" }] | ||
| requires-python = ">=3.10" | ||
| requires-python = ">=3.6" |
There was a problem hiding this comment.
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.
setup.py says python_requires=">=3.6", I've just transferred this to pyproject.toml.
devtools/create_tag.sh
Outdated
| # 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.
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.
I've created a dedicated ticket. Thank you.
There was a problem hiding this comment.
Why not fix it? Seems like things are sort of broken as they are in this PR
There was a problem hiding this comment.
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.
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.
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.
I might be seeing the problem now: Is it hatchling vs. hatch? So, let's just add hatch, right! Fixed with 8a9a172.
There was a problem hiding this comment.
Wonderful. I've just rebased this patch after you've resolved other concerns. Thank you again.
amotl
changed the title
setup.py. Improve Python compatibility.There was a problem hiding this comment.
I wouldn't re-add support for python < 3.10. They're all EOL (https://devguide.python.org/versions/)
Users of older python versions can use older versions of crate-python. Otherwise we could also add a note like "If you need support for older Python versions', concact ..." to get info about demand. If there isn't any, this would only be wasting CI cycles - and we would have to update the CI matrix if we want to claim the versions are supported.
And we should also mention it in the release notes that they're dropped
Hi. I it is not about re-adding support. We didn't start to drop support on them yet, because the recent changes have not been released. |
It isn't and wasn't covered in CI anymore given it was removed in commits like b143f1c We can't claim to support older versions without having it covered in CI. And given that the version's are EOL I don't see why we'd bother continuing to support them. Anybody using them is already taking on security risks, so they should also be fine running old versions of crate-python. #769 adds the missing release notes entry about dropping support |
3 participants
The project's code is in principle compatible with Python 3.6, or even earlier versions.