search

rehanguha / brisque

Blind/Referenceless Image Spatial QUality Evaluator (BRISQUE)
https://pypi.org/project/brisque/
Apache License 2.0
55 stars 8 forks source link

Make OpenCV requirements more permissive #14

Open charlienewey-odin opened 2 months ago

charlienewey-odin commented 2 months ago

This change allows a user to specify which version of opencv they wish to use.

As such this now supports opencv-python, opencv-python-headless, etc, etc.

This is a requirement in my company, hence the PR.

I hope you don't mind - I need to use this package immediately, so I have forked this repo and published a copy (with reference to the original!) here: https://pypi.org/project/brisque-opencv/

If this PR gets merged then I will get rid of the fork :grin:

rehanguha commented 2 months ago

@charlienewey-odin ... Thanks for the PR. It's a logical PR and everything looks great except the vanilla installation. If someone wants to install pip install brisque I feel it should install with opencv-python. Else the package might break for many who have incorporated in their requirements.txt

charlienewey-odin commented 2 months ago

Hi @rehanguha, thanks for taking a look at this so quickly.

One other thing to note is that I had to change from brisque import BRISQUE to from brisque.brisque import BRISQUE because of the way setup.py would import the version number from the package's __init__.py. This may also be a slightly disruptive change. I have updated the README to reflect this but I perhaps should have called attention to it when I opened the MR.

Re: the other issue - i.e. sensible defaults - I agree with you but there is no supported way to do this with setup.py or pyproject.toml which is a little frustrating, because it seems this is a common issue with opencv-* libraries.

Apparently there is an implementation of this kind of fallback logic in e.g. albumentations (see here) which permits this sort of "default package" behaviour but I could not get it to work as expected with testing.

rehanguha commented 2 months ago

Hi @charlienewey-odin, Yes that's a disruptive change 🙂. I am a bit occupied with my calendar this month so I might not get time to work on this module. If you have any luck cracking the default package problem. It will be great, nothing like it. For now let your package offer this flexibility of different opencv installation. I will pick up this issue soon and try to work something out.

Thanks again.