strange import desiutil.iers side effect

[sbailey]

@weaverba137 not urgent, but a curious puzzle:

Some desi code postpones calling desiutil.iers.freeze_iers() until after it is sure that it needs to, e.g. after parsing arguments to avoid the INFO log message and small delay if someone had just called desi_blah --help. In the case of desi_proc, it was even postponing import desiutil.iers, though I'm not sure if that was helpful at some point or just a leftover "optimization". However, this has the strange side effect of making python lose track of other desiutil imports which had happened earlier in the code. I haven't tracked if this is somehow unique to desiutil.iers or some other python package importing side effect. Examples:

This fails with "UnboundLocalError: local variable 'desiutil' referenced before assignment":

#!/usr/bin/env python
  
import desiutil.log

def main():
    # This fails with
    # UnboundLocalError: local variable 'desiutil' referenced before assignment
    log = desiutil.log.get_logger()
    log.info('hello')

    # the failure is somehow caused by this import within a function, even
    # though it occurs later in the function
    import desiutil.iers
    desiutil.iers.freeze_iers()
    log.info('goodbye')

if __name__ == "__main__":
    main()

But moving the import desiutil.iers to a standard package level import works:

#!/usr/bin/env python
  
import desiutil.log
import desiutil.iers

def main():
    log = desiutil.log.get_logger()
    log.info('hello')

    desiutil.iers.freeze_iers()
    log.info('goodbye')

if __name__ == "__main__":
    main()

The workaround is simple enough, to always import desiutil.iers at the package level instead of within a function, but it is a strange enough side effect that I'm filing a ticket for investigation. @weaverba137 could you take a look?

Related: in other cases desi_blah --help is noticeably more responsive by parsing arguments before importing heavyweight packages like numpy/scipy/astropy/matplotlib . If that's a bad pattern I'd like to understand the exact thing to avoid.

[weaverba137]

This is weird, but I think the problem is actually with desiutil.log, rather than desiutil.iers. If I replace import desiutil.log with from desiutil.log import get_logger and adjust log = get_logger() accordingly, then the script succeeds.

[weaverba137]

This also works:

from desiutil.log import log  # calls get_logger() at import time

Funny enough, the idiom that I prefer, where you only import what you need from a particular module, rather than the whole module, works better here.

[weaverba137]

And this also works:

#!/usr/bin/env python

def main():
    import desiutil.log
    log = desiutil.log.get_logger()
    log.info('hello')

    # the failure is somehow caused by this import within a function, even
    # though it occurs later in the function
    import desiutil.iers
    desiutil.iers.freeze_iers()
    log.info('goodbye')

if __name__ == "__main__":
    main()

So basically, if one puts all the imports in the same place, whether at the module level or the function level, it works. Putting all the imports at module level is preferred Python style anyway.

What is the specific use case for importing desiutil.log at module level, but only ever importing desiutil.iers at function level?

[schlafly]

This is roughly:
https://stackoverflow.com/questions/64141797/python-lazy-submodule-imports-invalidate-parent-modules-unboundlocalerror-b

[sbailey]

The original problem was discovered by @dmargala in desihub/desispec#1242 with importing desiutil.timer, i.e. the problem isn't specific to desiutil.log . I also haven't checked whether it is somehow specific to desiutil.freeze_iers.

import desiutil.iers takes nearly a second at NERSC, so a modest motivation for importing it later is to make a package import faster, and only import something if it hits code that really needs it.

But more to the point is that this is a case of something that seems like it should be a style preference (where to put imports, whether to "import blat.foo" or "from blat import foo") but is actually a requirement for how they have to be imported. I'd like to understand why that is actually a requirement and whether this is a python gotcha or whether we are doing something fancy under the hood that is restricting our import options.

[schlafly]

FWIW, my understanding is that it's a python gotcha/requirement. import x.y makes a new local variable x within the current scope, so earlier references to x within that scope trigger UnboundLocalErrors. I had definitely stumbled into this before without understanding it, and I don't yet feel confident saying that you can never use past global imports of the form import x.z if you later call import x.y, but that's my current understanding.

[weaverba137]

Having read the link that @schlafly sent, I think this is really is a case where style preference saves the day. To summarize: when evaluating the body of a function, it looks like assignments may in some cases be evaluated before imports. I strongly encourage @sbailey to read that article.

[sbailey]

Thanks @weaverba137 @schlafly; this does appear to be a python issue rather than anything specific to our packages. There may still be cases where we purposefully want to do imports within functions instead of at the package level, but at least know we understand the potential gotchas. Nothing needed on the desiutil side so closing ticket.