desihub/speclite

Imports in Python 3

weaverba137 opened this issue · 6 comments

speclite does not correctly import internal packages in Python 3.

For example in __init__.py:

if not _ASTROPY_SETUP_:
    from redshift import redshift
    from accumulate import accumulate
    from resample import resample
    from downsample import downsample
    import filters

For Python 3 support, this should be:

if not _ASTROPY_SETUP_:
    from .redshift import redshift
    from .accumulate import accumulate
    from .resample import resample
    from .downsample import downsample
    from . import filters

Do the current imports actually fail in py3? I guess my unit tests don't catch that -- so much for 100% coverage! How did you catch this?

For reference, here is the relevant section of PEP8.

Yes, the imports failed during tests of desispec. The problem with the PEP8 guide is that the definition of "absolute import" has changed in Python 3. Try running that same code in Python 2, but with from __future__ import absolute_import.

@moustakas I would like to take this opportunity to remove the import filters line from __init__.py. I took a quick look at desisim and I don't think this will break anything there. Specifically, speclite.filters is only imported in desisim.templates and always using:

from speclite import filters

I found one instance of import speclite in desispec.io.filters, so I will create a PR to fix that.

Are there other desihub packages I should check?

As far as I know just desisim.filters and the flux-calibration piece of desispec use the filter stuff.

I just fixed the relative imports and removed import filters on master.