equinor/seismic-zfp

pip install fails on ubuntu 18.04

fmell opened this issue · 4 comments

fmell commented

Installing seismic-zfp with pip on ubuntu 18.04 fails due to the zgy2sgz dependency:

$ pip3 install seismic-zfp

...

Collecting zgy2sgz (from seismic-zfp)
  Could not find a version that satisfies the requirement zgy2sgz (from seismic-zfp) (from versions: )
No matching distribution found for zgy2sgz (from seismic-zfp)

This dependency is not needed for segy and sgz files. Could this be changed to an optional dependency?

yes, this should definitely be an optional dependency!

In the ZgyConverter class' init you could do something like:

try:
    import zgy2sgz
    ... continue with the initialization
except ImportError:
    raise NotImplementedError("You can't use this wihtout the zgy2sgz package)

Not 100% sure how to handle this with requirements.txt / seutp.py though!

Can you add what version of pip you're using here? (default Ubuntu version or an upgrade?)

fmell commented

This is with the default pip version installed by apt (sudo apt install python3-pip)
python version: 3.6.9
pip version: 9.0.1

I tried upgrading pip with pip to 20.2.1. This works, but I think it is not recommended to do this.

fmell commented

A possible way to solve this would be to add the zgy2sgz as a "extra" in the setup.py.

In the converter.py we could have this import:

try:
    import zgy2sgz
except ImportError:
    _has_zgy2sgz = False
else:
    _has_zgy2sgz = True

In the ZgyConverter class init we could do this

def __init__(self, file, filetype_checking=True, preload=False, chunk_cache_size=None):
    if not _has_zgy2sgz:
        raise ImportError("zgy2sgz is required for SgzConverter. Install optional dependency seismic-zfp[zgy] with pip.")
    super().__init__(file, filetype_checking, preload, chunk_cache_size)

And in the setup.py we add this:

extras_require={
    'zgy': ['zgy2sgz']
},

This would let the user install seismic-zfpas normal with pip, but without the zgy2sgz dependency. To install seismic-zfp with zgy support you would install seismic-zfp[zgy].

See #29 for an example.

Yes, I think this is the right way to go about solving the issue. Will continue the discussion on #29 .