Skip to content

Conversation

@malfet
Copy link
Contributor

@malfet malfet commented Jul 1, 2022

To make project PEP-517 compliant

Solves pytorch/pytorch#80745

Copy link
Contributor

@datumbox datumbox left a comment

Choose a reason for hiding this comment

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

LGTM, thanks @malfet

@malfet malfet merged commit 7547180 into main Jul 1, 2022
@malfet malfet deleted the malfet-patch-1 branch July 1, 2022 19:09
@github-actions
Copy link

github-actions bot commented Jul 1, 2022

Hey @malfet!

You merged this PR, but no labels were added. The list of valid labels is available at https://github.com/pytorch/vision/blob/main/.github/process_commit.py


[build-system]

requires = ["setuptools", "torch", "wheel"]
Copy link
Contributor

Choose a reason for hiding this comment

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

Adding torch as a requirement here does only somewhat solve this:

  1. pip will create a separate virtual env for these requirements, which means it will pull torch from PyPI. That means you will get a CPU version for macOS and Windows and an outdated CUDA version (10.2 IIRC) for Linux.
  2. We only have the stable binaries up on PyPI, but in general we depend on the current nightly.

Thus, even with this set, users could have problems building from source.

I don't have a solution for this. Just raising awareness.

facebook-github-bot pushed a commit that referenced this pull request Jul 6, 2022
Summary:
To make project https://peps.python.org/pep-0517/ compliant

Solves pytorch/pytorch#80745

Reviewed By: jdsgomes

Differential Revision: D37643905

fbshipit-source-id: 6f20b9c38a18ed9c0c189ef377a39b88716998fe
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants