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 cv2 graphics / headless extras to setup.py #2775

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

Erotemic
Copy link

@Erotemic Erotemic commented Apr 22, 2023

Motivation

The mmcv library requires the cv2 module, but there are two flavors of the module that different users may want in different circumstances, either opencv-python or opencv-python-headless. These libraries are incompatible with each other, and if you incorrectly install the wrong one it becomes a pain to clean them up and install the right one.

This PR introduces a way for the user to specify which one of these they want at install time.

Modification

I've added two new files in requirements. cv2-headless.txt and cv2-graphics.txt which specify reasonable minimum versions of opencv based on the version of Python the user is on. These are registered as new extras_require options. So the new usage would be either:

pip install mmcv[cv2-headless]

OR

pip install mmcv[cv2-graphics]

This means you no longer have to install cv2 before / after mmcv, and you can be sure you have the right one for your use case based on the install command you use. I've been using this construct in kwimage and other cv repos for a while now with good success.

This also makes a modification to the parse_requirements function in setup.py. I was the original author of this construct and I figured I would update it to the lastest version I'm using in my cookiecutter repo while I was here. I don't think it matters to parse the new extra reqs, but it does support more cases that could exist in the future. That could be reverted if needbe.

Notes

I think to finialize this PR there will need to be documentation modification and maybe this section:

try:
    # OpenCV installed via conda.
    import cv2  # NOQA: F401
    major, minor, *rest = cv2.__version__.split('.')
    if int(major) < 3:
        raise RuntimeError(
            f'OpenCV >=3 is required but {cv2.__version__} is installed')
except ImportError:
    # If first not installed install second package
    CHOOSE_INSTALL_REQUIRES = [('opencv-python-headless>=3',
                                'opencv-python>=3')]
    for main, secondary in CHOOSE_INSTALL_REQUIRES:
        install_requires.append(choose_requirement(main, secondary))

In the setup.py needs to change? Looking for maintainer feedback about this detail.

Checklist

Before PR:

  • I have read and followed the workflow indicated in the CONTRIBUTING.md to create this PR.
  • Pre-commit or linting tools indicated in CONTRIBUTING.md are used to fix the potential lint issues.
  • Bug fixes are covered by unit tests, the case that causes the bug should be added in the unit tests.
  • New functionalities are covered by complete unit tests. If not, please add more unit test to ensure the correctness.
  • The documentation has been modified accordingly, including docstring or example tutorials.

After PR:

  • If the modification has potential influence on downstream or other related projects, this PR should be tested with some of those projects, like MMDet or MMCls.
  • CLA has been signed and all committers have signed the CLA in this PR.

@CLAassistant
Copy link

CLAassistant commented Apr 22, 2023

CLA assistant check
All committers have signed the CLA.

@Erotemic
Copy link
Author

I'm not sure why the windows / osx dashboards failed. I don't think any change I've introduced here would cause that. I've rebased on a current main to see if updating helps.

Having this change would be helpful because currently pip complains about package conflicts if python-opencv-headless is installed instead of python-opencv. Here is a recent error message I've gotten:

ERROR: pip's dependency resolver does not currently take into account all the packages that are installed. This behaviour is the source of the following dependency conflicts.
mmengine 0.7.4 requires opencv-python>=3, which is not installed.
mmcv 2.0.0 requires opencv-python>=3, which is not installed.

It might require this change be ported to mmengine as well.

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.

None yet

2 participants