iancovert / sage

For calculating global feature importance using Shapley values.
MIT License
255 stars 34 forks source link

Add support for `pyproject.toml`πŸ†• #23

Closed KarelZe closed 10 months ago

KarelZe commented 1 year ago

This PR adds support for a pyproject.toml-based project setup, which is recommended over setup.py, as stated here. Sister projects like shap have adopted a similar setup.

In the future, it will allow us to configure the project as well as tools such as linters, formatters, type checkers etc in a single file.

Changes:

@iancovert Could you please review?

PS: Are you open to a PR for a linter/formatter e.g., ruff?

iancovert commented 1 year ago

Thanks for suggesting this! I've been meaning to read about the different options for building the package, I've noticed that many others use this approach. I need a little time to look into this more - from your changes it looks like you know what you're doing here, but I should probably understand a bit better before merging it in.

KarelZe commented 12 months ago

Thanks for your feedback.😊 Regarding building: Currently, the github action is just trying to build the package using the information form pyproject.toml, but not preserving the build artifacts. Regarding building and publishing on (test) pypi, I could alter/extend the GitHub action, so that whenever you create a release on GitHub (other triggers possible), a new release is published on (test) pypi. Also, I'll preserve the build artifacts. The process is well explained here: https://packaging.python.org/en/latest/guides/publishing-package-distribution-releases-using-github-actions-ci-cd-workflows/ I'll update my pr to make the idea more clearπŸ‘

KarelZe commented 11 months ago

@iancovert I extended the PR, so that it supports automatic releases to pypi and test-pypi.

The workflow is based on this python guide: https://packaging.python.org/en/latest/guides/publishing-package-distribution-releases-using-github-actions-ci-cd-workflows/. Whenever you push to GitHub a release on test-pypi is created/updated for testing. For tagged commits (git tag 0.0.6 + git push --tag) a new GitHub release is created + a release on pypi. You may want to bump the version number in pyproject.toml before making a new release.

⚠️ In order for this to work, you need to set up trusted publishing (see previous guide) + create an account/project at test.pypi.org.

More over, I added a configuration for the dependabot to keep actions up-to-date.

KarelZe commented 10 months ago

@iancovert Did you have the chance to look into this yet?

iancovert commented 10 months ago

@KarelZe apologies for the delay, I was busy with a deadline that finally passed last week. Thanks again for putting this PR together, I think these are important improvements for the package. Separately, this was a good forcing function for me to learn about these things, and I spent most of today learning about pyproject.toml files, setting up pre-commit hooks, and using GitHub Actions to automatically publish to PyPI.

I've wrapped my head around most of the changes you introduced here. The one exception is the workflow files, but I imagine some of this is boilerplate you found in a tutorial or in similar packages? I'm comfortable merging this and seeing if the publishing action works properly, so I'll go ahead and do that.

KarelZe commented 9 months ago

No worries.πŸ€“ I'm very happy, that you found the changes useful and that everything went smoothly with the release.

The workflow file is an adaption of https://docs.github.com/en/actions/automating-builds-and-tests/building-and-testing-python (test if builds are possible for various python versions) and https://packaging.python.org/en/latest/guides/publishing-package-distribution-releases-using-github-actions-ci-cd-workflows/ (create gh release + deploy to pypi), but tailored to the repo. Dependabot is useful to keep gh actions, dependencies, etc. up-to-date.

I might put together a small PR in the next days for a linter / formatter configuration in the pyproject.toml / .pre-commit-config, if you like.

iancovert commented 9 months ago

Yes everything is working great with the new changes, thanks again for all your help.

And re: linting/formatting that would be great, I'd like to see how you set this up! I saw the ruff pre-commit hook in this repo of yours so maybe that's all it takes, but I'm curious to see.

KarelZe commented 9 months ago

@iancovert I prepared a PR for formatting/linting (#26) this morning.