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

fix(packaging): pyproject.toml #208

Merged
merged 1 commit into from
Feb 20, 2024
Merged

Conversation

juftin
Copy link
Contributor

@juftin juftin commented Feb 12, 2024

This PR resolves #204

This change declares torch as a build dependency for this package since it's used in the setup.py - see setuptools build system support.

Here is the new pyproject.toml file:

[build-system]
requires = ["setuptools", "torch"]
build-backend = "setuptools.build_meta"

Once this is done pip will understand that torch is a build dependency and this package can be installed from PyPI and an ImportError will no longer be raised. When you pip install pytorch-cluster the package will now build itself from source as part of its installation process.

Testing

The below command uses the Python Package Authority's build package to test the fix. I was able to build wheels for x86 and ARM (M2) MacOS after this change (using Rosetta for x86):

pip install build
python -m build
> ...
> Successfully built torch_cluster-1.6.3.tar.gz and torch_cluster-1.6.3-cp311-cp311-macosx_13_0_arm64.whl

@codecov-commenter
Copy link

Codecov Report

All modified and coverable lines are covered by tests ✅

Comparison is base (e0eb0c1) 97.74% compared to head (d54b049) 97.74%.

❗ Your organization needs to install the Codecov GitHub app to enable full functionality.

Additional details and impacted files
@@           Coverage Diff           @@
##           master     #208   +/-   ##
=======================================
  Coverage   97.74%   97.74%           
=======================================
  Files          11       11           
  Lines         222      222           
=======================================
  Hits          217      217           
  Misses          5        5           

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@juftin juftin changed the title fix(pep-517): pyproject.toml fix(packaging): pyproject.toml Feb 12, 2024
@rusty1s
Copy link
Owner

rusty1s commented Feb 13, 2024

Thanks. This looks good, I only have one concern: How does this work if PyTorch is not installed? I would assume PyTorch is then installed with a default CUDA version, which leads to issues downstream if the local CUDA version doesn't match the one of torch from PyPi.

@juftin
Copy link
Contributor Author

juftin commented Feb 13, 2024

Thanks. This looks good, I only have one concern: How does this work if PyTorch is not installed? I would assume PyTorch is then installed with a default CUDA version, which leads to issues downstream if the local CUDA version doesn't match the one of torch from PyPi.

Since torch is a build-time dependency in this change and not a runtime dependency - it will only be installed during the build. But you're right, it will use the default CUDA. So currently if you're running linux it would install torch with cuda 12.1 during the build.

I'm not actually sure if differing versions of CUDA are significant during build time vs. runtime. I believe if you're wanting a different CUDA than the default you would still want to install the binary using the --find-link URL as specified in the README.

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.

setup.py depends on torch
3 participants