pysal / pointpats

Planar Point Pattern Analysis in PySAL
https://pysal.org/pointpats/
BSD 3-Clause "New" or "Revised" License
83 stars 26 forks source link

pointpats on conda-forge fails pip check #80

Closed giswqs closed 1 year ago

giswqs commented 2 years ago

Conda-forge now recommends adding pip check in the recipe. The pointpats package on the conda-forge channel fails pip check with an error pointpats 2.2.0 requires opencv-contrib-python, which is not installed. This is causing issues for all downstream packages that might be submitted to the conda-forge channel in the future. It appears that opencv-contrib-python in the setup.py, but it is not available on the conda-forge channel.

https://github.com/conda-forge/pointpats-feedstock/blob/b433cecfc45129bae5fe956f11f0b7b20efa339d/recipe/meta.yaml#L21

giswqs commented 2 years ago

Steps to reproduce the error:

conda install -c conda-forge pointpats
pip check
ljwolf commented 2 years ago

Hi @giswqs! The odd thing is that, I think, the package does not require opencv. It's a "soft dependency," being that one function attempts to import it, but if the package is not provided, then the function simply errors. This pattern is used elsehwere in python (such as pandas.read_excel) and is pretty common... I will need to read up on how pip check works in order to resolve the issue.

giswqs commented 2 years ago

The opencv-contrib-python package is listed on the requirements.txt, which is causing the issue. If you remove it from requirements.txt and check the dependency in a module and prompt users to install them if needed, that should solve the issue.

giswqs commented 2 years ago

It appears that the only function using opencv is minimum_rotated_rectangle(). Since the function already provides error handling, I think opencv-contrib-python can be safely removed from requirements.txt. Otherwise, it will cause issues for all pysal downstream packages on conda-forge with pip check. Also, opencv-contrib-python is quite a large package (150MB). including such a large package as a required dependency that is only used by one function is not ideal.

https://github.com/pysal/pointpats/blob/9a09bd7ff2c3b5402f02a0a54175193cb643edd8/pointpats/centrography.py#L105-L108

martinfleis commented 1 year ago

I believe that this was resolved by #82.