Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Switch to pyproject.toml #917

Merged
merged 5 commits into from
Sep 4, 2024
Merged

Conversation

adamjstewart
Copy link
Collaborator

This PR replaces the aging setup.py and MANIFEST.in with a modern pyproject.toml. For comparison, see TorchGeo's pyproject.toml.

So far this includes no dependency changes. In my next PR I want to add tests for a range of dependency versions so we know which versions of timm are and aren't supported.

@adamjstewart
Copy link
Collaborator Author

NOTE: It's possible to dynamically read from requirements.txt, but in the future I'm planning on using pyproject.toml to list the actual range of dependencies and requirements.txt to list the latest tested version. So dependabot controls the latter, and the latter is used in CI, but the former is used for actually installing smp.

@qubvel
Copy link
Collaborator

qubvel commented Sep 2, 2024

Thanks for working on this! Looks good to me, the only question is how to safely manage both lists of requirements, isn't it better to have only one list with ranges to test and install the same versions? Maybe I'm missing something :)

@adamjstewart
Copy link
Collaborator Author

The idea with versions is that we don't want the same versions for both test and install.

At install time, we want to support the widest range of versions possible. If a new version of timm comes out, we want people to be able to use it immediately. We don't want to force people to use one exact version, because if every library did that, nothing could be installed.

This comes at a cost. It becomes much harder to ensure that all of these versions are actually compatible. So at test time, we want to test both the minimum supported version, and the latest version. We keep these pinned to a specific version so that user PRs don't fail due to random changes in dependencies. Then, dependabot updates these pinned versions so that if something does break, it happens in a separate PR and is easy to debug and fix.

I'm not sure if I've ever seen a good article about this, this is mostly from experience. The first paragraph partly comes from:

The second paragraph comes more from experience. For a long time, TorchGeo was the only library I knew that tested both minimum supported versions and the latest version. Since then, matplotlib has also been doing the same thing. The idea is very similar to testing a matrix of Python versions, except now we're also testing a matrix of dependencies too.

Copy link
Collaborator

@qubvel qubvel left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ok, thanks a lot for the explanation! Let's try this approach

@qubvel
Copy link
Collaborator

qubvel commented Sep 4, 2024

Btw, are you able to merge a PR now?

@adamjstewart adamjstewart merged commit 25a6bd2 into qubvel-org:main Sep 4, 2024
2 checks passed
@adamjstewart adamjstewart deleted the pyproject-toml branch September 4, 2024 08:34
@adamjstewart
Copy link
Collaborator Author

It looks like I am!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants