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 dependencies #146

Merged
merged 4 commits into from
Nov 21, 2019
Merged

fix dependencies #146

merged 4 commits into from
Nov 21, 2019

Conversation

DXist
Copy link
Contributor

@DXist DXist commented Oct 30, 2019

This PR changes install_requires.

  1. Dependency on opencv-python is resolved in the same way as done in albumentationshttps://github.com/albu/albumentations/blob/master/setup.py#L12) library. opencv-python is used if it has already been installed. Otherwise opencv-python-headless is used which is good for minimal container environments.

  2. Cython is build-time dependency and is removed from install_requires

@codecov
Copy link

codecov bot commented Oct 30, 2019

Codecov Report

Merging #146 into master will not change coverage.
The diff coverage is n/a.

Impacted file tree graph

@@          Coverage Diff           @@
##           master    #146   +/-   ##
======================================
  Coverage    39.5%   39.5%           
======================================
  Files          67      67           
  Lines        2724    2724           
======================================
  Hits         1076    1076           
  Misses       1648    1648

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 8829ff0...5705b06. Read the comment docs.

setup.py Outdated

dist.Distribution().fetch_build_eggs(['Cython', 'numpy>=1.11.1'])

import numpy # noqa: E402
from Cython.Distutils import build_ext # noqa: E402


def choose_requirement(main, secondary):
"""If some version version of main requirement installed, return main,
Copy link
Member

Choose a reason for hiding this comment

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

A minor typo here: version version.

setup.py Outdated
]
# If first not installed install second package
CHOOSE_INSTALL_REQUIRES = [('opencv-python>=4.1.1',
Copy link
Member

Choose a reason for hiding this comment

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

We may relax the version requirement for opencv. OpenCV 3 & 4 should both work well.

@hellock
Copy link
Member

hellock commented Nov 9, 2019

Thanks for the fix!

@hellock
Copy link
Member

hellock commented Nov 17, 2019

Hi @DXist, I changed the priority of opencv-python and opencv-python-headless.
MMCV provides lots of visualization methods, which requires the GUI support by default. If we install opencv-python-headless by default, there will be errors raised in new environments.
To guide the users who meet issues related to the dependency, I add some comments in the readme.
Is it ok to you?

@DXist
Copy link
Contributor Author

DXist commented Nov 17, 2019

Hi @hellock . It's ok for me but there would be inconsistency with albumentations order that is used by mmdetection.

If mmdetection and mmcv are installed by a single pip install command then both opencv-python and opencv-python-headless will be installed, one of them will overwrite another one.

@hellock
Copy link
Member

hellock commented Nov 18, 2019

I see, so the installation guide in mmdetection should be the following to avoid installing both opencv-python and opencv-python-headless.

pip install mmcv
# git clone #
pip install -e .

If users would like to switch to opencv-python-headless, then it should be installed first.

@DXist
Copy link
Contributor Author

DXist commented Nov 18, 2019

@hellock sounds reasonable

@hellock hellock merged commit aea7500 into open-mmlab:master Nov 21, 2019
@GiscardBiamby
Copy link

Hi, if I install opencv-python-headless first, and then run

pip install mmcv

Then it also installs opencv-python and i end up with both versions installed. This gives me errors when I try to run python test.py for example.

Any suggestion on how to install mmcv so that it detects that I have the headless library already installed and avoids installing the non-headless?

@DXist
Copy link
Contributor Author

DXist commented Dec 21, 2019

There is inconsistency: albumentations checks for regular opencv-python version and installs headless variant by default, mmcv does the opposite: checks for headless version and installs regular one by default.

Visualization can be viewed as an extra feature. If someone needs it then he/she could install regular opencv-python and related dependencies.
I think opencv-python-headless is a good default with less dependencies.

@OpenMMLab-Assistant-004
Copy link

Hi @DXist !First of all, we want to express our gratitude for your significant PR in the mmcv project. Your contribution is highly appreciated, and we are grateful for your efforts in helping improve this open-source project during your personal time. We believe that many developers will benefit from your PR.

We would also like to invite you to join our Special Interest Group (SIG) private channel on Discord, where you can share your experiences, ideas, and build connections with like-minded peers. To join the SIG channel, simply message moderator— OpenMMLab on Discord or briefly share your open-source contributions in the #introductions channel and we will assist you. Look forward to seeing you there! Join us :https://discord.gg/raweFPmdzG

If you have WeChat,welcome to join our community on WeChat. You can add our assistant :openmmlabwx. Please add "mmsig + Github ID" as a remark when adding friends:)
Thank you again for your contribution❤

1 similar comment
@OpenMMLab-Assistant-004
Copy link

Hi @DXist !First of all, we want to express our gratitude for your significant PR in the mmcv project. Your contribution is highly appreciated, and we are grateful for your efforts in helping improve this open-source project during your personal time. We believe that many developers will benefit from your PR.

We would also like to invite you to join our Special Interest Group (SIG) private channel on Discord, where you can share your experiences, ideas, and build connections with like-minded peers. To join the SIG channel, simply message moderator— OpenMMLab on Discord or briefly share your open-source contributions in the #introductions channel and we will assist you. Look forward to seeing you there! Join us :https://discord.gg/raweFPmdzG

If you have WeChat,welcome to join our community on WeChat. You can add our assistant :openmmlabwx. Please add "mmsig + Github ID" as a remark when adding friends:)
Thank you again for your contribution❤

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.

4 participants