update setup.py to pyproject.toml

[ds-cbo]

Pull Request check-list

• Do tests and lints pass with this change?
• Do the CI tests pass with this change (enable it first in your forked repo and wait for the github action build to finish)?
• Is the new or changed code fully tested?
• Is a documentation update included (if this change modifies existing APIs, or introduces new ones)?
• Is there an example added to the examples folder (if applicable)?

Description of change

I upgraded the deprecated setup.py to the modern pyproject.toml format, enabling modern installer tools.

[codecov-commenter]

Codecov Report

Attention: Patch coverage is 0% with 2 lines in your changes missing coverage. Please review.

Project coverage is 75.16%. Comparing base (c8c19c2) to head (fcb7b67).
Report is 3 commits behind head on main.

Files with missing lines	Patch %	Lines
setup.py	0.00%	2 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main      #96      +/-   ##
==========================================
+ Coverage   75.12%   75.16%   +0.04%     
==========================================
  Files         132      132              
  Lines       34398    34480      +82     
==========================================
+ Hits        25840    25918      +78     
- Misses       8558     8562       +4     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

[mkmkme]

Thanks for your contribution.

One question though: why do you think setup.py is deprecated? This web page explicitly says it is not.

[ds-cbo]

Interesting, those articles contradict what I see other big projects doing, even if they keep using setuptools as build system:

• pypa/pip@de77de5
• django/django@4686541
• pallets/flask@6d6d986
• ansible/ansible@68515ab
• sphinx-doc/sphinx@5477ef6 + sphinx-doc/sphinx@b347657

Internally we also moved to pyproject by removing setup.py because we believed that was the new way to go, I didn't even know that you could keep both files in tandem.

Would you like me to bring back a barebones from setuptools import setup ; setup() setup.py?

[aiven-sal]

Did you encounter any specific issues with setup.py or is this intended more as a cosmetic change?

[aiven-sal]

BTW yes, please don't remove setup.py completely.

As a side note, the sign-off thingy should contain a real name and a real email address

[ds-cbo]

@aiven-sal I encountered issues with our build system which requires pyproject.toml be available in libraries we use. Of course we could also put some patches in our repo only, but I imagined it might be useful to more people to contribute upstream too.

I've added the setup.py in the last commit.

What do you mean with "real name" and "real email address"? These are the two linked to this GitHub account, I imagined those would be more useful than some generic untraceable values.

[aiven-sal]

What do you mean with "real name" and "real email address"? These are the two linked to this GitHub account, I imagined those would be more useful than some generic untraceable values.

Maybe this can clarify what I mean: https://github.com/cncf/foundation/blob/main/dco-guidelines.md#dco-and-real-names
The "real name" doesn't necessarily have to be the name on your ID, but it shouldn't be an "anonymous id" and the email needs to be an actual email address that can be used to contact you directly.

[aiven-sal]

To put it in a different way. Imagine we just met at a conference, what name would you use to introduce yourself? That should be the name you use in signoff

[ds-cbo]

I guess that would be it then?

[mkmkme]

The CI has failed. Could you please fix it accordingly?

It seems that:

1. Words pyproject and toml should be added to the dictionary
2. The CI is using python setup.py directly which is no longer correct

[ds-cbo]

@mkmkme I've just updated the dictionary, but as far as I know I already replaced all instances of python setup.py from CI in a previous commit. Am I still missing something (and if so: where?), or were you perhaps looking at an older CI run?

Do you also expect me to pass the (seemingly unrelated and/or outdated?) pip-audit checks?

[mkmkme]

@ds-cbo There's still a bunch of leftovers that you can find by https://github.com/search?q=repo%3Avalkey-io%2Fvalkey-py%20setup.py&type=code

IMO all of them should be fixed.
The one that is causing the CI to fail is under .github/workflows. The ones in INSTALL and tasks.py are user-facing, so I'd change them, too.

Thanks! :)

[mkmkme]

Do you also expect me to pass the (seemingly unrelated and/or outdated?) pip-audit checks?

We certainly can't merge the PR with CI failing. If you don't want to handle this, I can do it myself.

[mkmkme]

I am also quite concerned that the direct calls to python setup.py from the CI are failing despite the fact we do have setup.py now

[ds-cbo]

@ds-cbo There's still a bunch of leftovers that you can find by https://github.com/search?q=repo%3Avalkey-io%2Fvalkey-py%20setup.py&type=code

IMO all of them should be fixed. The one that is causing the CI to fail is under .github/workflows. The ones in INSTALL and tasks.py are user-facing, so I'd change them, too.

Thanks! :)

As far as I know, I've already modified those in earlier commits. See for example https://github.com/ds-cbo/valkey-py/blob/pyproject.toml/.github/workflows/install_and_test.sh#L23

We certainly can't merge the PR with CI failing. If you don't want to handle this, I can do it myself.

Understandable. I wouldn't mind fixing it, but I don't really know your preferred course of action here. It's failing on Babel 2.8.0 even though 2.16.0 is already available and nothing seems to prevent it from being installed. Maybe sphinx<7.0 in our dependency list is blocking it? The last sphinx<7.0 would be 6.2.1 from april 2023. I have no clue what the impact of that major upgrade would be on this project though, so I don't want to mindlessly remove the upper pin.

I'm guessing this change happened because now we're running pip-audit including the docs dependencies, where previously we'd only run it on dev? I'm a bit unsure

I am also quite concerned that the direct calls to python setup.py from the CI are failing despite the fact we do have setup.py now

I still think you're looking at older CI reports, the reason the newest packaging runs were failing was because build wasn't available, and I've just pushed a commit to fix that.