desihub / speclite

Lightweight utilities for working with spectroscopic data
14 stars 19 forks source link

Imports in Python 3 #23

Closed weaverba137 closed 8 years ago

weaverba137 commented 8 years ago

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
dkirkby commented 8 years ago

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?

dkirkby commented 8 years ago

For reference, here is the relevant section of PEP8.

weaverba137 commented 8 years ago

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.

dkirkby commented 8 years ago

@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?

moustakas commented 8 years ago

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

dkirkby commented 8 years ago

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