Implement packaging using `pyproject.toml` #476

abhi8893 · 2024-05-03T10:06:59Z

Description

scikit-surprise currently relies on the legacy setup.py to describe the python package build. While this is needed for including cythonized extensions, most of the config can be moved to pyproject.toml.

This would probably solve open installation issues: #466, #468, #470 etc

This PR does the following:

add build to dev requirements
remove setup.cfg
document version directly in surprise.__init__.py
create minimal required setup.py, and transfer most conf to pyproject.toml
create sdist using build, test on sdist with [test] requirements installed

Merging this will gave back the community a really good recommender system library! 🙂

….toml

…alled

pshelby · 2024-05-07T14:03:52Z

@NicolasHug I'm currently having trouble installing Surprise with Poetry. This would be a great improvement! Thanks @abhi8893 !

bhatiaakshay8 · 2024-05-08T07:47:46Z

@NicolasHug This solved the problem for me! Please merge and make a new release.

thanks @abhi8893

rodrigc · 2024-05-08T21:44:53Z

I didn't realize @abhi8893 came up with this, and implemented something very similar in #477.
However, this PR is slightly better, so I closed #477 in favor of this.

I only have some minor suggestions for this PR, please consider incorporating.

rodrigc · 2024-05-08T21:47:52Z

pyproject.toml

+  "Programming Language :: Python :: 3.9",
+  "Programming Language :: Python :: 3.10",
+]
+license = { "text" = "GPLv3+" }


Suggested change

license = { "text" = "GPLv3+" }

license = { "file" = "LICENSE.md" }

The license is not GPLv3+, so just point it to the file.

rodrigc · 2024-05-08T21:51:39Z

surprise/__init__.py

@@ -1,5 +1,3 @@
-from pkg_resources import get_distribution


Suggested change

from pkg_resources import get_distribution

from pkg_resources import get_distribution

See discussion here in this PR: #476 (comment)

rodrigc · 2024-05-08T21:52:08Z

surprise/__init__.py

@@ -1,5 +1,3 @@
-from pkg_resources import get_distribution
-


Suggested change

See discussion here in this PR: #476 (comment)

rodrigc · 2024-05-08T21:52:34Z

surprise/__init__.py

@@ -47,4 +45,4 @@
    "model_selection",
 ]

-__version__ = get_distribution("scikit-surprise").version


Suggested change

__version__ = get_distribution("scikit-surprise").version

__version__ = get_distribution("scikit-surprise").version

Add back get_distribution(). I don't think you want to hardcode the version number here.

Here, there are 2 considerations:

If we rely on pkg_resources (which comes from setuptools) inside surprise, then it becomes a direct dependency of surprise. Depending on the python distribution, setuptools may or may not be bundled with python. Hence to avoid this, I declare version directly in surprise.__init__.py

Most popular packages, like scikit-learn follow the same style: https://github.com/scikit-learn/scikit-learn/blob/main/sklearn/__init__.py

Let me know if this makes sense 🙂

OK, thanks for the explanation. That makes sense. 👍
In that case, I would recommend changing the docstring in setup.py
in the section Change __version__ to reflect that the version is now in __init__.py.

This is a valid change, because in pyproject.toml you specified that version is dynamic.

rodrigc · 2024-05-08T21:56:44Z

You can delete requirements.txt and requirements_dev.txt since you copied those dependencies into pyproject.toml

setup.py

…ect.toml

…e in a single step

abhi8893 · 2024-05-09T13:09:26Z

Thanks @rodrigc for reviewing! I have incorporated the comments. On the versioning part, I have a differing view, but let me know what you think 🙂

dhpancor · 2024-05-11T10:09:59Z

Good job on this PR.
@NicolasHug please merge and make a new release.

NicolasHug · 2024-05-19T12:30:53Z

@abhi8893 thank you so much for putting out this PR
@rodrigc thank you so much as well for your previous PR and for reviewing this one.

I've pushed a bunch of minor changes but I think it's good to merge now. I'll try to push 1.1.4 soon.
🙏

abhi8893 · 2024-05-19T16:28:02Z

Thanks a lot for merging this and making a new release so quickly @NicolasHug! Confirming it installs flawlessly now, even with uv ! 🚀

rodrigc · 2024-05-19T19:53:29Z

Nice job @abhi8893 and @NicolasHug !

abhi8893 added 10 commits May 3, 2024 15:10

add build to dev requirements

1ac85b1

remove setup.cfg

2a14cfd

document version directly in surprise.__init__.py

06910f7

create minimal required setup.py, and transfer most conf to pyproject…

19711b2

….toml

create sdist using build, test on sdist with [test] requirements inst…

2714221

…alled

remove unused imports in setup.py

c58d1a2

fix formatting issues in setup.py

826854e

fix sdist name for scikit_surprise

3b77eec

change artifact paths for upload and download in ga

91b6e94

remove test ls statements from ga

9c7dbc8

rodrigc mentioned this pull request May 8, 2024

Add pyproject.toml file to support Python 3.12 #477

Closed

rodrigc suggested changes May 8, 2024

View reviewed changes

rodrigc reviewed May 8, 2024

View reviewed changes

setup.py Show resolved Hide resolved

rodrigc mentioned this pull request May 8, 2024

Compatibility with Python 3.12 #473

Closed

abhi8893 added 5 commits May 9, 2024 17:55

change license reference to file in pyproject.toml

404d152

update instructions in setup.py for changing numpy versions in pyproj…

fb368aa

…ect.toml

alter build_docs ga workflow to install docs dependencies and surpris…

e0fa767

…e in a single step

clean lint workflow to only install lint dependencies

a14242c

remove requirements.txt and requirements_dev.txt

6a33e1b

NicolasHug added 5 commits May 17, 2024 15:47

Update dependencies upper bounds

8222b72

Use build --sdist

1f6bcd1

use oldest-supported-numpy

0dfb376

Update CI jobs to build one sdist per Python version

3f03c3e

Hardcode version number in build_sdist.yml because I'm paranoid

fbe5b1f

NicolasHug added 14 commits May 17, 2024 18:06

Fix

ae703d6

Fix Benchmarks?

b1190c8

Cleanup setup.py

893b020

cleanup pyproject.toml

421c1f2

Actually only create a single sdist, on 3.8

047e715

Ensure there are no C files in sdist

be2b53d

No comma?

96aef98

wtf

be5da73

AJNFJFEA

bba6f10

Fix

321aa84

Fix?

c5bd275

ugh?

9129a15

whatever

b966108

last

5fc5ae5

NicolasHug approved these changes May 19, 2024

View reviewed changes

NicolasHug merged commit 79bcd39 into NicolasHug:master May 19, 2024
9 checks passed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Implement packaging using `pyproject.toml` #476

Implement packaging using `pyproject.toml` #476

abhi8893 commented May 3, 2024 •

edited

pshelby commented May 7, 2024

bhatiaakshay8 commented May 8, 2024 •

edited

rodrigc commented May 8, 2024

rodrigc May 8, 2024 •

edited

rodrigc May 8, 2024

abhi8893 May 9, 2024

rodrigc May 8, 2024

abhi8893 May 9, 2024

rodrigc May 8, 2024

abhi8893 May 9, 2024

rodrigc May 8, 2024

rodrigc May 8, 2024

abhi8893 May 9, 2024

rodrigc May 9, 2024

rodrigc commented May 8, 2024 •

edited

abhi8893 commented May 9, 2024

dhpancor commented May 11, 2024

NicolasHug commented May 19, 2024

abhi8893 commented May 19, 2024

rodrigc commented May 19, 2024

	license = { "text" = "GPLv3+" }
	license = { "file" = "LICENSE.md" }

	from pkg_resources import get_distribution
	from pkg_resources import get_distribution

	__version__ = get_distribution("scikit-surprise").version
	__version__ = get_distribution("scikit-surprise").version

Implement packaging using pyproject.toml #476

Implement packaging using pyproject.toml #476

Conversation

abhi8893 commented May 3, 2024 • edited

Description

pshelby commented May 7, 2024

bhatiaakshay8 commented May 8, 2024 • edited

rodrigc commented May 8, 2024

rodrigc May 8, 2024 • edited

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

Choose a reason for hiding this comment

Choose a reason for hiding this comment

rodrigc commented May 8, 2024 • edited

abhi8893 commented May 9, 2024

dhpancor commented May 11, 2024

NicolasHug commented May 19, 2024

abhi8893 commented May 19, 2024

rodrigc commented May 19, 2024

Implement packaging using `pyproject.toml` #476

Implement packaging using `pyproject.toml` #476

abhi8893 commented May 3, 2024 •

edited

bhatiaakshay8 commented May 8, 2024 •

edited

rodrigc May 8, 2024 •

edited

rodrigc commented May 8, 2024 •

edited