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

Add torch to build requirements #682

Merged
merged 2 commits into from
Nov 10, 2021

Conversation

r-b-g-b
Copy link
Contributor

@r-b-g-b r-b-g-b commented Sep 10, 2021

Since setup.py imports torch

import torch

installing yolox requires a separate install of torch beforehand (as documented in the README). However, there is a standard way to include build dependencies for a Python package. From the PyPA docs:

pyproject.toml tells build tools (like pip and build) what is required to build your project.

Adding pyproject.toml with a build-system.requires section that includes torch would eliminate the need to install torch with a separate command prior to installing yolox. (Importantly, this only installs torch during the build stage, it still needs to be installed into the Python environment to use the package). This makes it easier to use a library in other packages.

For example, the following fails:

pip install git+https://github.com/Megvii-BaseDetection/YOLOX.git

Collecting git+https://github.com/Megvii-BaseDetection/YOLOX.git
  Cloning https://github.com/Megvii-BaseDetection/YOLOX.git to /tmp/pip-req-build-wruzfplt
  Running command git clone -q https://github.com/Megvii-BaseDetection/YOLOX.git /tmp/pip-req-build-wruzfplt
  Resolved https://github.com/Megvii-BaseDetection/YOLOX.git to commit 6819425e78e28926d132863d4be20811729c9d44
    ERROR: Command errored out with exit status 1:
     command: /home/robert/anaconda3/envs/zamba-package/bin/python -c 'import io, os, sys, setuptools, tokenize; sys.argv[0] = '"'"'/tmp/pip-req-build-wruzfplt/setup.py'"'"'; __file__='"'"'/tmp/pip-req-build-wruzfplt/setup.py'"'"';f = getattr(tokenize, '"'"'open'"'"', open)(__file__) if os.path.exists(__file__) else io.StringIO('"'"'from setuptools import setup; setup()'"'"');code = f.read().replace('"'"'\r\n'"'"', '"'"'\n'"'"');f.close();exec(compile(code, __file__, '"'"'exec'"'"'))' egg_info --egg-base /tmp/pip-pip-egg-info-dveu39rs
         cwd: /tmp/pip-req-build-wruzfplt/
    Complete output (5 lines):
    Traceback (most recent call last):
      File "<string>", line 1, in <module>
      File "/tmp/pip-req-build-wruzfplt/setup.py", line 8, in <module>
        import torch
    ModuleNotFoundError: No module named 'torch'
    ----------------------------------------

Whereas this succeeds:

pip install git+https://github.com/r-b-g-b/YOLOX.git@build-requirements

Collecting git+https://github.com/r-b-g-b/YOLOX.git@build-requirements
  Cloning https://github.com/r-b-g-b/YOLOX.git (to revision build-requirements) to /tmp/pip-req-build-z748qonp
  Running command git clone -q https://github.com/r-b-g-b/YOLOX.git /tmp/pip-req-build-z748qonp
  Running command git checkout -b build-requirements --track origin/build-requirements
  Switched to a new branch 'build-requirements'
  Branch 'build-requirements' set up to track remote branch 'build-requirements' from 'origin'.
  Resolved https://github.com/r-b-g-b/YOLOX.git to commit 3631ad70d1b01d5988ff5745655eb2ff182d5bd9
  Installing build dependencies ... done
  Getting requirements to build wheel ... done
    Preparing wheel metadata ... done

@r-b-g-b
Copy link
Contributor Author

r-b-g-b commented Oct 21, 2021

@FateScript Any thoughts on this? I'd love to use YOLOX as a library in our package. Thanks!

@CLAassistant
Copy link

CLAassistant commented Nov 5, 2021

CLA assistant check
All committers have signed the CLA.

@FateScript
Copy link
Member

Sorry for not not noticing this PR. LGTM. Thanks for contributing @r-b-g-b

@FateScript FateScript merged commit 2c2dd13 into Megvii-BaseDetection:main Nov 10, 2021
@r-b-g-b r-b-g-b deleted the build-requirements branch November 10, 2021 14:53
FateScript added a commit that referenced this pull request Nov 18, 2021
Joker316701882 pushed a commit that referenced this pull request Nov 18, 2021
Githubinme pushed a commit to Githubinme/YOLOX that referenced this pull request Jun 6, 2023
Githubinme pushed a commit to Githubinme/YOLOX that referenced this pull request Jun 6, 2023
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.

3 participants