Surprise icon indicating copy to clipboard operation
Surprise copied to clipboard
verified publisher icon NicolasHug

Implement packaging using `pyproject.toml`

Open abhi8893 opened this issue 1 year ago • 5 comments

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:

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

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

abhi8893 avatar May 03 '24 10:05 abhi8893

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

pshelby avatar May 07 '24 14:05 pshelby

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

thanks @abhi8893

bhatiaakshay8 avatar May 08 '24 07:05 bhatiaakshay8

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 avatar May 08 '24 21:05 rodrigc

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

rodrigc avatar May 08 '24 21:05 rodrigc

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 🙂

abhi8893 avatar May 09 '24 13:05 abhi8893

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

dhpancor avatar May 11 '24 10:05 dhpancor

@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. :pray:

NicolasHug avatar May 19 '24 12:05 NicolasHug

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

abhi8893 avatar May 19 '24 16:05 abhi8893

Nice job @abhi8893 and @NicolasHug !

rodrigc avatar May 19 '24 19:05 rodrigc