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 installation instructions #60

Closed
johnbradley opened this issue Oct 15, 2024 · 6 comments · Fixed by #68
Closed

Fix installation instructions #60

johnbradley opened this issue Oct 15, 2024 · 6 comments · Fixed by #68
Assignees
Labels
enhancement New feature or request

Comments

@johnbradley
Copy link

When installing kabr-tools in a clean environment as described in the README:

pip install git+https://github.com/Imageomics/kabr-tools

The following error occurs:

...
Collecting detectron2@ git+https://github.com/facebookresearch/detectron2.git@2a420edb307c9bdf640f036d3b196bed474b8593 (from kabr_tools==0.1.0)
...
ModuleNotFoundError: No module named 'torch'
@johnbradley
Copy link
Author

The issue is that our detectron2 dependency uses the torch library as part of setup.py:
https://github.com/facebookresearch/detectron2/blob/8d85329aed8506ea3672e3e208971345973ea761/setup.py#L10-L11
So before we can install detectron2 pytorch must be installed.

Perhaps the simplest solution is to change our README to specify that pytorch is a requirement and reference the detectron2 docs: https://detectron2.readthedocs.io/en/latest/tutorials/install.html#requirements

Potential new installation instructions:

pip install torch torchvision
pip install git+https://github.com/Imageomics/kabr-tools

@johnbradley
Copy link
Author

johnbradley commented Oct 15, 2024

Even with the above steps on a Mac I still hit the error.
It did print a more helpful instruction:

  Caused by: This error likely indicates that detectron2 @ git+https://github.com/facebookresearch/detectron2.git@2a420edb307c9bdf640f036d3b196bed474b8593 
depends on torch, but doesn't declare it as a build dependency.
 If detectron2 @ git+https://github.com/facebookresearch/detectron2.git@2a420edb307c9bdf640f036d3b196bed474b8593 is a first-party package, consider adding torch to its `build-system.requires`. 
Otherwise, `uv pip install torch` into the environment and re-run with `--no-build-isolation`.

Unfortunately --no-build-isolation doesn't seem to work with hatchling our current build system. Adding "torch" to build-system-requires still has the same error. Perhaps we could switch to setuptools.

@zhong-al
Copy link
Collaborator

zhong-al commented Oct 18, 2024

Can you check if enhancement/setuptools fixes this? python -m pip install git+https://github.com/Imageomics/kabr-tools@138d170e7b5ed9ce76b3846d2f01b8d015cfa608 works for me (just tried with a new conda environment on Mac).

@johnbradley
Copy link
Author

@zhong-al After deleting the ~/.cache directory install from this branch still fails for me. Even adding 'torch' to build-system.requires in addition to your changes doesn't fix the issue. I think instead we need to update the README to include two steps for installation of the software: 1) Install pytorch 2) install kabr-tools. We could reference the detectron2 Requirements in our README:https://detectron2.readthedocs.io/en/latest/tutorials/install.html#requirements. Alternatively we could referencing the pytorch documentation: https://pytorch.org/get-started/locally/. On the pytorch page users can pick the version of CUDA and other options based on their environment.

@zhong-al
Copy link
Collaborator

zhong-al commented Oct 21, 2024

@johnbradley I updated the instructions here. I can update it again if it doesn't fix the issue. Should I close the setuptools PR since it fails to fix the issue?

@johnbradley
Copy link
Author

@zhong-al These instructions look good to me. The instructions worked fine for me using pip within a conda environment on my Mac.

I still had a problem trying to use uv pip after deleting ~/.cache/uv/, but I think that's a problem with uv: astral-sh/uv#2252.

I would close the setuptools PR - there are quite a few more changes necessary to switch over to setuptools. Since it doesn't fix this issue I'm not aware of any advantage we would get to switching from hatch to setuptools.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
3 participants