Migration to `pyproject.toml` #3338

hirosassa · 2025-01-19T07:11:35Z

Description

In this PR, I migrated from setup.py to pyproject.toml using uv.
Fixes #3327.

In detail I did followings:

migrate setup.py features to equivalent pyproject.toml
introduce uv as a package manager
move dependency configurations for test from tox.ini to pyproject.toml
update tox version to use new features of tox and pyproject.toml
drop Python 3.7 support. see: Migration to pyproject.toml #3338 (comment)

A part of codes are burrowed from @dlstadther 's branch.
ref master...dlstadther:luigi:migrate-setup-to-pyproject

Motivation and Context

python setup.py install is deprecated.

Have you tested this? If so, how?

CI worked.

dlstadther

Awesome start on this! I really appreciate your active involvement. My time is too unpredictable to spend significant dedicated time to this project.

RELEASE-PROCESS.rst

dlstadther · 2025-01-19T21:39:44Z

pyproject.toml

+common = [
+  "pytest<7.0",


I have 2 preferences here related to sorting:

keep package names in alphabetic order

keep group names in alphabetic order

This makes it easiest for humans to view and also to maintain order where new packages should be added.

fixed in 9616953

dlstadther · 2025-01-19T21:42:58Z

pyproject.toml

+[project.optional-dependencies]
+jsonschema = ["jsonschema"]
+prometheus = ["prometheus-client>=0.5,<0.15"]
+toml = ["toml<2.0.0"]
+
+[dependency-groups]
+dropbox = [


(This comment doesn't need to be addressed as part of this PR; mostly just thinking aloud)

Perhaps it makes sense in the future to move some of the dependency-groups content which is used for unit test execution for specific luigi modules (particularly the contrib and optional stuff) out of dependency-groups and into project.optional-dependencies to facilitate luigi installation with "extras". These extras can also easily control the cases where the luigi module may only support up to a max version.

Food for thought if it changes how you design/organize this file.

dlstadther · 2025-01-20T00:14:38Z

tox.ini

+requires =
+    tox>=4.22 # `dependency_groups` needed
+    tox-uv>=1.19


Could you help me understand why these requirements are here and not in pyproject?

In general case, tox version is not managed in pyproject.toml, requirements.txt, etc.
https://github.com/tox-dev/tox-uv/blob/main/.github/workflows/check.yaml#L40
https://github.com/tox-dev/tox-uv/blob/main/tox.ini#L2-L4
I think this is because tox is not related to actual implementation of tests.

In our CI, we use dependency_groups feature of pyproject.toml and tox, so I added this requirements on tox's configuration.

dlstadther · 2025-01-20T00:15:45Z

doc/conf.py

@@ -67,7 +67,10 @@ def _warn_node(self, msg, node, *args, **kwargs):

 sphinx.environment.BuildEnvironment.warn_node = _warn_node

-if os.environ.get('READTHEDOCS', None) == 'True':
+# on_rtd is whether we are on readthedocs.org, this line of code grabbed from docs.readthedocs.org


Before merging, we'll need to identify how to verify the merging of the final version of this branch doesn't break Luigi's readthedocs.

@hirosassa , how you thought how we can be sure that we don't break RTD when merging this PR?

I think it is a little bit difficult because it depends on webhook of this repo.
I confirmed that a local build of documents using Sphinx is succeeded.

hirosassa · 2025-02-01T08:35:48Z

@dlstadther I confirmed CIs are all green without Python 3.7.
see: https://github.com/spotify/luigi/actions/runs/13086373164/job/36517994352

With Python 3.7, test fails in code below:
https://github.com/mysql/mysql-connector-python/blob/8.0.33/lib/mysql/connector/connection_cext.py#L291
https://github.com/spotify/luigi/actions/runs/13086478927/job/36518207022?pr=3338#step:6:166

I think the root cause of this problem is Python 3.7.9 runtime.
The succeeded case runs in 3.7.17.
https://github.com/spotify/luigi/actions/runs/12875540263/job/35897024647#step:7:19

The reason why the CI in this PR for Python 3.7 run in 3.7.9 is uv supports only 3.7.9 for Python 3.7
We cannot use newer Python with uv because uv using python frompython-builds-standalone and it dropped Python 3.7.10+ support.
https://github.com/astral-sh/python-build-standalone/releases/tag/20210103

We might need to consider dropping support for Python 3.7.
Do you have any idea solving this problem?

RRap0so · 2025-02-01T22:02:55Z

@dlstadther I confirmed CIs are all green without Python 3.7.

see: https://github.com/spotify/luigi/actions/runs/13086373164/job/36517994352

With Python 3.7, test fails in code below:

https://github.com/mysql/mysql-connector-python/blob/8.0.33/lib/mysql/connector/connection_cext.py#L291

https://github.com/spotify/luigi/actions/runs/13086478927/job/36518207022?pr=3338#step:6:166

I think the root cause of this problem is Python 3.7.9 runtime.

The succeeded case runs in 3.7.17.

https://github.com/spotify/luigi/actions/runs/12875540263/job/35897024647#step:7:19

The reason why the CI in this PR for Python 3.7 run in 3.7.9 is uv supports only 3.7.9 for Python 3.7

We cannot use newer Python with uv because uv using python frompython-builds-standalone and it dropped Python 3.7.10+ support.

https://github.com/astral-sh/python-build-standalone/releases/tag/20210103

We might need to consider dropping support for Python 3.7.

Do you have any idea solving this problem?

With 3.7 already EOL for awhile I think it's fine dropping support.

dlstadther · 2025-02-02T01:52:57Z

Looking at https://pypistats.org/packages/luigi , there are still 3.7 downloads, but fewer than 3.8+. I'm in favor of removing 3.7 support

hirosassa · 2025-02-02T02:00:12Z

@dlstadther @RRap0so Thanks for your comment!
Let's drop Python 3.7.

RELEASE-PROCESS.rst

hirosassa · 2025-02-02T02:39:52Z

@dlstadther This PR is ready for review!

RRap0so · 2025-02-03T14:54:25Z

We probably need to drop the checks for 3.7 but I'm wondering why there's 3.8,3.9 even 3.10 checks not being reported?

@hirosassa can you take a look at the changes? Something is up, some checks haven't ran.

I can remove the 3.7 checks once this is good to go.

hirosassa · 2025-02-03T20:56:47Z

@RRap0so Thank you for your comment!
This is because I changed environment matrixes changes, for example for simplifying environment specification.
So we should follow repository setting of "must pass CI".
You can fix this on settings > branch > master > edit > Require a pull request before merging > Status checks that are required.

dlstadther

A few comments/questions.

Also, we'll need to have the merge approval settings updated by @RRap0so .

dlstadther · 2025-02-07T02:13:37Z

.github/workflows/pythonbuild.yml

      - name: Build
        env:
          TOXENV: ${{ matrix.tox-env }}
          OVERRIDE_SKIP_CI_TESTS: ${{ matrix.OVERRIDE_SKIP_CI_TESTS }}
-        run: tox
+        run: uvx --with tox-uv tox run


The step just below this line includes a conditional to not run coverage upload if flake8 or docs. But this "others" matrix only includes flake8 and docs. Could we then remove its Upload coverage to Codecov step entirely?

fixed in 48650b7

dlstadther · 2025-02-07T02:15:42Z

RELEASE-PROCESS.rst

@@ -1,13 +1,14 @@
 For maintainers of Luigi, who have push access to pypi. Here's how you upload
 Luigi to pypi.

-#. Make sure [twine](https://pypi.org/project/twine/) is installed ``pip install twine``.
-#. Update version number in `luigi/__meta__.py`.
+#. Make sure [uv](https://github.com/astral-sh/uv) is installed ``curl -LsSf https://astral.sh/uv/install.sh | sh``.


Here we suggest to install uv with curl+sh, but in contributing.rst, we suggest pipx. We should be consistent between the two docs. Personally, i prefer curl+sh because it doesn't require the installation of another tool to install uv.

fixed in 6f8a76f

dlstadther · 2025-02-07T02:17:33Z

doc/conf.py

@@ -67,7 +67,10 @@ def _warn_node(self, msg, node, *args, **kwargs):

 sphinx.environment.BuildEnvironment.warn_node = _warn_node

-if os.environ.get('READTHEDOCS', None) == 'True':
+# on_rtd is whether we are on readthedocs.org, this line of code grabbed from docs.readthedocs.org


@hirosassa , how you thought how we can be sure that we don't break RTD when merging this PR?

dlstadther · 2025-02-07T02:21:19Z

.github/workflows/pythonbuild.yml

      - name: Install dependencies
        run: |
-          python -m pip install --upgrade pip 'tox<4.0'
+          uv tool install --python-preference only-managed --python 3.12 tox --with tox-uv


I'm a bit confused how the CI is working know to use the correct python version for each tox environment. Could you help me understand why we use py 3.12 to install tox, but expect the correct python version to run the specific tox environment tests? Are there details within tox-uv that handle this?

Are there details within tox-uv that handle this?

Yes, you are right.
ref: https://github.com/tox-dev/tox-uv

if you run uvx --with tox-uv tox -e py311 then we can see the log below:

$ uvx --with tox-uv tox -e py311 py311: uv-sync> uv sync --frozen --python-preference system --no-dev -p cpython3.11 ...

At the first line of the log shows constructing process of Python 3.11 environment.

hirosassa added 16 commits January 18, 2025 17:14

migration to pyproject.toml

e4cddf2

migrate to pyproject.yml

b575804

migrate to pyproject.yml

0c7f24d

Merge branch 'master' into migration-to-uv

87de330

apply {envpython}

aee8e9c

apply {envpython}

06df545

pass env parameter directly

5ad74bc

remove cache tentatively

0816898

remove 3.8 tentatively

2e88012

allowlist_externals

a62ef47

allowlist_externals

d51c3b0

fix back

1ce319b

update requires

48b9029

enforce tox-uv versions

e486da0

enforce tox-uv versions

c9e2a8f

change versions

3e0720d

hirosassa requested review from dlstadther and a team as code owners January 19, 2025 07:11

hirosassa changed the title ~~Migration to uv~~ Migration to pyproject.toml Jan 19, 2025

hirosassa marked this pull request as draft January 19, 2025 07:15

hirosassa added 4 commits January 19, 2025 17:34

remove unused variables

f9819be

remove constraint on sqlalchemy

483aa25

refactor workflow

62f195a

remove runner

cb3d1c1

hirosassa force-pushed the migration-to-uv branch from 6c32e93 to cb3d1c1 Compare January 19, 2025 10:02

hirosassa added 2 commits January 19, 2025 19:05

remove pi

aa22b4e

version constraint

5e68ba0

dlstadther reviewed Jan 20, 2025

View reviewed changes

hirosassa added 2 commits January 25, 2025 15:25

fix: docs runs on python 310

d60b65f

test: run python 3.8 or above

77a223d

hirosassa force-pushed the migration-to-uv branch from 81d9cae to 1fef047 Compare January 25, 2025 23:04

hack: use system python for py37

2821873

hirosassa force-pushed the migration-to-uv branch from 1fef047 to 2821873 Compare January 25, 2025 23:45

hirosassa added 3 commits January 26, 2025 08:49

fix back

62fd597

Merge branch 'master' into migration-to-uv

43c2856

use LuigiTestCase

844d94d

hirosassa force-pushed the migration-to-uv branch from 2ec1144 to 844d94d Compare February 1, 2025 04:02

hirosassa added 3 commits February 1, 2025 16:14

extend timeout tentatively

0038252

fix back

1e7b2dc

sort dependency groups and dependencies

9616953

hirosassa added 2 commits February 2, 2025 07:08

update release process document

12a9d8a

Merge branch 'master' into migration-to-uv

7be8a6d

hirosassa force-pushed the migration-to-uv branch 2 times, most recently from 9d5181b to 99ac166 Compare February 2, 2025 02:15

drop Python 3.7 support

cd6fee3

hirosassa force-pushed the migration-to-uv branch from 99ac166 to cd6fee3 Compare February 2, 2025 02:24

hirosassa marked this pull request as ready for review February 2, 2025 02:28

hirosassa commented Feb 2, 2025

View reviewed changes

RELEASE-PROCESS.rst Show resolved Hide resolved

hirosassa requested a review from dlstadther February 3, 2025 20:56

dlstadther reviewed Feb 7, 2025

View reviewed changes

hirosassa added 2 commits February 7, 2025 11:50

removed unnecessary codecov step

48650b7

fix install process of uv

6f8a76f

hirosassa requested a review from dlstadther February 7, 2025 05:57

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Migration to `pyproject.toml` #3338

Migration to `pyproject.toml` #3338

hirosassa commented Jan 19, 2025 •

edited

Loading

dlstadther left a comment

dlstadther Jan 19, 2025

hirosassa Feb 1, 2025

dlstadther Jan 19, 2025

dlstadther Jan 20, 2025

hirosassa Feb 1, 2025

dlstadther Jan 20, 2025

dlstadther Feb 7, 2025

hirosassa Feb 7, 2025

hirosassa commented Feb 1, 2025 •

edited

Loading

RRap0so commented Feb 1, 2025

dlstadther commented Feb 2, 2025

hirosassa commented Feb 2, 2025

hirosassa commented Feb 2, 2025

RRap0so commented Feb 3, 2025 •

edited

Loading

hirosassa commented Feb 3, 2025 •

edited

Loading

dlstadther left a comment

dlstadther Feb 7, 2025

hirosassa Feb 7, 2025

dlstadther Feb 7, 2025

hirosassa Feb 7, 2025

dlstadther Feb 7, 2025

dlstadther Feb 7, 2025

hirosassa Feb 7, 2025 •

edited

Loading

		common = [
		"pytest<7.0",

Migration to pyproject.toml #3338

Are you sure you want to change the base?

Migration to pyproject.toml #3338

Conversation

hirosassa commented Jan 19, 2025 • edited Loading

Description

Motivation and Context

Have you tested this? If so, how?

dlstadther left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

hirosassa commented Feb 1, 2025 • edited Loading

RRap0so commented Feb 1, 2025

dlstadther commented Feb 2, 2025

hirosassa commented Feb 2, 2025

hirosassa commented Feb 2, 2025

RRap0so commented Feb 3, 2025 • edited Loading

hirosassa commented Feb 3, 2025 • edited Loading

dlstadther left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

hirosassa Feb 7, 2025 • edited Loading

Choose a reason for hiding this comment

Migration to `pyproject.toml` #3338

Migration to `pyproject.toml` #3338

hirosassa commented Jan 19, 2025 •

edited

Loading

hirosassa commented Feb 1, 2025 •

edited

Loading

RRap0so commented Feb 3, 2025 •

edited

Loading

hirosassa commented Feb 3, 2025 •

edited

Loading

hirosassa Feb 7, 2025 •

edited

Loading