Catch errors on import

[banesullivan]

A potential solution to #73

This has a bare except which is frowned upon. Also the test I added here will only work if you have pyvips improperly installed. We may want a better test or to scrap this test

[codecov-commenter]

Codecov Report

Merging #74 (33fd060) into main (f351a07) will increase coverage by 0.11%. The diff coverage is 100.00%.

@@            Coverage Diff             @@
##             main      #74      +/-   ##
==========================================
+ Coverage   86.38%   86.50%   +0.11%     
==========================================
  Files           4        4              
  Lines         360      363       +3     
==========================================
+ Hits          311      314       +3     
  Misses         49       49              

[emixa-d]

At least on Python 3.9.9, the exception is a ModuleNotFoundError, not an OSError (see https://issues.guix.gnu.org/55060#20). Maybe it should test for ModuleNotFoundError as well?

[banesullivan]

Thanks for pointing this out @emixa-d! Is this still an issue for what you're doing in 0.5.12? The bare except, while not great, should handle any issues on import of a package

[emixa-d]

Thanks for pointing this out @emixa-d! Is this still an issue for what you're doing in 0.5.12? The bare except, while not great, should handle any issues on import of a package

I'm not sure what you mean here? The problem was detected in version 0.5.12 (and not some earlier version), whose test suites failed in Guix, according to the patch submitter, so there are some problems with packaging it. So still an issue AFAICT.

Also, while perhaps the bare except should handle issues, apparently it doesn't, as the patch submitter had to change the OSError to a ModuleNotFoundError to make the tests pass.

I'm not doing anything in 0.5.12 or any other version of scooby, ‘Paul A. Patience’ from https://issues.guix.gnu.org/55060#0 is -- I only did some reviewing, I'm not using scooby myself.

[banesullivan]

That patch is invalid for what the test it is changing is supposed to evaluate.

Sure, if a package is not installed, it will generally raise a ModuleNotFoundError error on import.

However pyvips is listed in the requirements_test.txt for testing:

https://github.com/banesullivan/scooby/blob/999d9ca46e1f747c7b835d6ee4814bb6e9118de6/requirements_test.txt#L10

And pip installing pyvips without installing the binaries (through apt, or the package manager of the system) will, indeed raise an OSError on import.

Here is an example where first I try importing it when not installed and see the ModuleNotFoundError, but then do pip install pyvips without install the binaries and indeed get an OSError:

(dev) ➜  scooby git:(main) python
Python 3.8.4 | packaged by conda-forge | (default, Jul 17 2020, 14:54:34)
[Clang 10.0.0 ] on darwin
Type "help", "copyright", "credits" or "license" for more information.
>>> import pyvips
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
ModuleNotFoundError: No module named 'pyvips'
>>> exit()
(dev) ➜  scooby git:(main) pip install pyvips
Processing /Users/bane/Library/Caches/pip/wheels/d5/be/bd/3406e16ca8f9f15088e28e8acace902cbce6da410194f99231/pyvips-2.2.0-py2.py3-none-any.whl
Collecting cffi>=1.0.0
  Using cached cffi-1.15.0-cp38-cp38-macosx_10_9_x86_64.whl (178 kB)
Collecting pycparser
  Using cached pycparser-2.21-py2.py3-none-any.whl (118 kB)
Installing collected packages: pycparser, cffi, pyvips
Successfully installed cffi-1.15.0 pycparser-2.21 pyvips-2.2.0
(dev) ➜  scooby git:(main) python
Python 3.8.4 | packaged by conda-forge | (default, Jul 17 2020, 14:54:34)
[Clang 10.0.0 ] on darwin
Type "help", "copyright", "credits" or "license" for more information.
>>> import pyvips
Traceback (most recent call last):
  File "/Users/bane/anaconda3/envs/dev/lib/python3.8/site-packages/pyvips/__init__.py", line 19, in <module>
    import _libvips
ModuleNotFoundError: No module named '_libvips'

During handling of the above exception, another exception occurred:

Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
  File "/Users/bane/anaconda3/envs/dev/lib/python3.8/site-packages/pyvips/__init__.py", line 71, in <module>
    vips_lib = ffi.dlopen(_vips_libname)
  File "/Users/bane/anaconda3/envs/dev/lib/python3.8/site-packages/cffi/api.py", line 150, in dlopen
    lib, function_cache = _make_ffi_library(self, name, flags)
  File "/Users/bane/anaconda3/envs/dev/lib/python3.8/site-packages/cffi/api.py", line 832, in _make_ffi_library
    backendlib = _load_backend_lib(backend, libname, flags)
  File "/Users/bane/anaconda3/envs/dev/lib/python3.8/site-packages/cffi/api.py", line 827, in _load_backend_lib
    raise OSError(msg)
OSError: cannot load library 'libvips.42.dylib': dlopen(libvips.42.dylib, 0x0002): tried: '/Users/bane/anaconda3/envs/dev/lib/libvips.42.dylib' (no such file), '/Users/bane/anaconda3/envs/dev/lib/libvips.42.dylib' (no such file), '/Users/bane/anaconda3/envs/dev/bin/../lib/libvips.42.dylib' (no such file), 'libvips.42.dylib' (no such file), '/usr/local/lib/libvips.42.dylib' (no such file), '/usr/lib/libvips.42.dylib' (no such file), '/Users/bane/Software/banesullivan/scooby/libvips.42.dylib' (no such file).  Additionally, ctypes.util.find_library() did not manage to locate a library called 'libvips.42.dylib'
>>>

The test in:

https://github.com/banesullivan/scooby/blob/999d9ca46e1f747c7b835d6ee4814bb6e9118de6/tests/test_scooby.py#L166-L172

ensures that this kind of scenario is handled by scooby.

A vanilla ModuleNotFoundError is tested by:

https://github.com/banesullivan/scooby/blob/999d9ca46e1f747c7b835d6ee4814bb6e9118de6/tests/test_scooby.py#L90-L91

[emixa-d]

Bane Sullivan schreef op vr 29-04-2022 om 20:33 [+0000]:

That patch is invalid for what the test it is changing is supposed to evaluate. Sure, if a package is not installed, it will generally raise a ModuleNotFoundError error on import. However pyvips is listed in the requirements_test.txt for testing: https://github.com/banesullivan/scooby/blob/999d9ca46e1f747c7b835d6ee4814bb6e9118de6/requirements_test.txt#L10 And pip installing pyvips without installing the binaries (through apt, or the package manager of the system) will, indeed raise an OSError on import.

Ok thanks, I added a note downstream to look into adding pyvips to the list of test dependencies. Looks like Guix' importer doesn't recognise requirements_test.txt yet ...

[banesullivan]

adding pyvips to the list of test dependencies

I do want to emphasize that pyvips is NOT a dependency of scooby itself, but only a dependency for testing a specific, but important edge case