Allow opencv-python as requirement if alread present

[fchouteau]

Referencing #473

This is a proposal, heavily inspired by a similar solution in albumentations that checks if opencv-python is installed when listing requirements and replaces opencv-python-headless if this is the case.

This is not the most "elegant" alternative, however I don't know any solution without resorting to extra requires and mandating the installation of imgaug with pip install imgaug[opencv-python] or pip install imgaug[opencv-python-headless]

Maybe extra_require installation would be preferred ? If so I can submit the PR again

[codecov-io]

Codecov Report

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

@@           Coverage Diff           @@
##           master     #586   +/-   ##
=======================================
  Coverage   96.48%   96.48%           
=======================================
  Files          40       40           
  Lines       14549    14549           
=======================================
  Hits        14037    14037           
  Misses        512      512

[fchouteau]

Note: I disabled C0114 for setup.py 391d71b

Writing """The setup file""" would be easier, but technically, is "setup.py" a module ? I can resubmit the PR with the module docstring if preferred

[aleju]

Hey, thanks for the patch. I suspect that having such a check in setup.py is overall the best approach to deal with the different cv2 packages. Having no OpenCV package as an explicit requirement and instead demanding a manual install, like suggested in #473, is probably great for professional users, but could easily confuse less experienced ones. Using "extra requires" looks elegant, but it seems like the concept can only be used to add additional requirements and not to remove them (e.g. imgaug[no-opencv] to use a custom install). I.e. imgaug would have to default to not having any opencv package as a requirement and that would again confuse inexperienced users when they try to install imgaug instead of imgaug[opencv-python]. It would potentially also break many existing systems that have imgaug as a requirement, expecting it to auto-install an opencv package, as it currently does.

The patch looks good to me. It would be great if it would be extended a bit to provide a list of alternative packages, instead of only a single one. That way opencv-contrib-python and opencv-contrib-python-headless could also be accepted. Not sure if there's a specific reason why the albumentations don't accept them -- maybe they just forgot about these packages.

Oh and you can ignore codecov complaining. That thing is currently only there to occasionally catch errors in the code. It will probably be removed in the future.

[aleju]

You could change this to # pylint: disable=missing-module-docstring

[aleju]

["opencv-python", "opencv-contrib-python", "opencv-contrib-python-headless"] here instead of just "opencv-python".

[aleju]

This should then expect alternative_install_require to be a list and iterate over it via a for loop.
If an alternative name in a loop iteration is found, return that name. Otherwise return install_require at the end.

[aleju]

I think the .keys() here is not necessary

[fchouteau]

Hello,
I agree with you on the fact that the package should be "sane by default" for most users (e.g. come with its own opencv requirements instead of using extra_require)

The PR was updated to account for multiple alternatives

[aleju]

This is now merged into master.
Thanks for the PR and the adding the changes!