pytorch3d icon indicating copy to clipboard operation
pytorch3d copied to clipboard

fix(setup): add torch as build system requirement

Open wsascha opened this issue 1 year ago • 4 comments

What?

I've added a new pyproject.toml file to the project that contains static information according to PEP 517.

I've also added torch as build system requirement so that isolated installation via setuptools (and also poetry and probably most other PEP 517 compliant build frontends) works.

Tests?

I've tested this via

pip install --isolated --verbose --editable .

from within the project directory (and via poetry when I provide this project as path dependency).

Anything else?

I should also say that I have no clue about any other specifics that might make the installation or packaging fail and just did this to solve my current problem of integrating pytorch3d into poetry. Anyway, I'm also interested in getting your feedback on this..

WDYT?

wsascha avatar Mar 16 '23 13:03 wsascha

Hi @wsascha!

Thank you for your pull request and welcome to our community.

Action Required

In order to merge any pull request (code, docs, etc.), we require contributors to sign our Contributor License Agreement, and we don't seem to have one on file for you.

Process

In order for us to review and merge your suggested changes, please sign at https://code.facebook.com/cla. If you are contributing on behalf of someone else (eg your employer), the individual CLA may not be sufficient and your employer may need to sign the corporate CLA.

Once the CLA is signed, our tooling will perform checks and validations. Afterwards, the pull request will be tagged with CLA signed. The tagging process may take up to 1 hour after signing. Please give it that time before contacting us about it.

If you have received this in error or have any questions, please contact us at [email protected]. Thanks!

facebook-github-bot avatar Mar 16 '23 13:03 facebook-github-bot

We were discussing this on #1419 . I don't think this is the right kind of change. But also I don't really understand the problem you are trying to solve. From my point of view, I generally want all users to pick their version of PyTorch manually.

bottler avatar Mar 16 '23 13:03 bottler

There's a difference between "I want users to pick their torch version manually" and "I need to have torch installed before I can even know what dependencies this library has". As far as I can see, your library's deps (and a lot of other project meta data) are not even dependent on a specific torch version. – They are just passed to the setup function call. So, I'm wondering why you don't define this project information in a separate file that can be read by modern tools (e.g. to build a conflict-free dependency tree) before running that setup.py to get the remaining information depending on the torch version.

Anyway, I figured there's currently a blocker for adding torch as build requirement as pip exactly does what you are afraid of – just installing the latest version of build requirements instead of choosing the already installed one: https://github.com/pypa/pip/issues/9542

I consider this a bug which is resolved sooner or later. If you'd still like me to make other project information available in a PEP 517 compliant way, I can adapt this pr. – let me know what you think.

wsascha avatar Mar 20 '23 23:03 wsascha

When you say you "consider this a bug", do you mean the behaviour of pip or this issue in pytorch3d? Unless someone can clearly explain how not to break things which are currently working, pytorch3d isn't going to change on this soon and there isn't a bug in pytorch3d. The discussion might as well be had on #1419 though, not here.

bottler avatar Mar 21 '23 15:03 bottler