scientific-python/lazy-loader

UnboundLocalError appeared in lazy-loader 0.4

Bingshuang0u0 opened this issue · 5 comments

I was using librosa.load() which uses lazy-loader, and the error below appeared.

Error Message:

UnboundLocalError: local variable 'parent' referenced before assignment

The Code Where The Error Appeared:

try:
    parent = inspect.stack()[1]
    frame_data = {
        "filename": parent.filename,
        "lineno": parent.lineno,
        "function": parent.function,
        "code_context": parent.code_context,
    }
    return DelayedImportErrorModule(
        frame_data,
        "DelayedImportErrorModule",
        message=not_found_message,
    )
finally:
    del parent

This was mentioned in issue#79, but it happened again in version 0.4

Environment:

Python 3.9.13
lazy-loader 0.4

Thanks for the report @Bingshuang0u0; I'll take a look.

I don't fully understand what this try-finally block is doing for us. It will delete before returning or raising, but I think the following should get you the same result, while allowing inspect.stack()[1] to fail:

# Can raise without triggering finally
parent = inspect.stack()[1]
# Can raise AttributeError
try:
    frame_data = {
        "filename": parent.filename,
        "lineno": parent.lineno,
        "function": parent.function,
        "code_context": parent.code_context,
    }
finally:
    del parent
# Parent is deleted, so we can safely raise anything here
return DelayedImportErrorModule(
    frame_data,
    "DelayedImportErrorModule",
    message=not_found_message,
)

That said, I don't think any of those attribute lookups on parent can fail. I think we could drop the try-finally and just:

parent = inspect.stack()[1]
frame_data = {
    "filename": parent.filename,
    "lineno": parent.lineno,
    "function": parent.function,
    "code_context": parent.code_context,
}
del parent
return DelayedImportErrorModule(
    frame_data,
    "DelayedImportErrorModule",
    message=not_found_message,
)

I agree that it is very unlikely that an exception will occur during the construction of the frame_data dict. It would only happen if the object on the inspect.stack was missing one of these attributes. That would mean something is seriously wrong.

Blame shows the finally clause was added in #4 presumably to remove the parent pointer to the object in inspect.stack()[1] when we return the DelayedImportErrorModule, or when an exception occurs while constructing that module. When reading the code today, I assumed the try/finally was looking to handle the case when an error occurs creating the module.

But of course the try is a few lines above where it needs to be for that. We definitely don't want the try to include errors during creating parent if the finally is deleting parent.

I don't know any reason to worry about deleting variable pointers like parent before returning other than garbage collection. And I'm not an expert on garbage collection in Python. But however it is handled, deleting parent before constructing and returning should have the same effect as the finally clause. And it removes the problem here where creating parent is inside the try/finally.

So, I think @effigies suggestion of removing the try/finally and adding del parent just before returning is a good solution.

Great, thanks for the suggestions y'all. Did anyone make a PR, or should I go ahead?

Sure, just opened #137.