Skip to content

Doubt about Absolute import vs relative import #128

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
skoudoro opened this issue Jul 30, 2024 · 9 comments
Open

Doubt about Absolute import vs relative import #128

skoudoro opened this issue Jul 30, 2024 · 9 comments

Comments

@skoudoro
Copy link

Hi all,

Our package is using absolute import (fury: https://github.com/fury-gl/fury).

We just added lazy-loader package and it seems to work like a charm.

However, when we activate EAGER_IMPORT on our CI's or locally, we encounter many circular import and everything fails.

Not sure why and it seems I miss understand something.

Can someone explain this behavior?

I check all the other projects using lazy-loader package and they all use relative import. So, I suppose, there is a reason I do not get or an issue with the package.

Thank you for the feedback

@stefanv
Copy link
Member

stefanv commented Aug 2, 2024

I'm away atm but please ping me if you don't hear back by mid-August.

@Erotemic
Copy link

I don't think this has anything to do with absolute vs relative imports. It looks like a problem with how EAGER_IMPORT was designed. When EAGER_IMPORT is truthy, it runs these lines inside the attach function:

    if os.environ.get("EAGER_IMPORT", ""):
        for attr in set(attr_to_modules.keys()) | submodules:
            __getattr__(attr)

which means that when this is called __getattr__ is not defined on the top level of the module, so any lazy attribute that attempts to re-access a top-level attribute will reference the base module before it's __getattr__ is defined, and thus not be able access the lazy variables.

The reason I've never run into this is that I don't think I typically write code where simply accessing an attribute will access another attribute in the module. But thinking about it, I'm not immediately sure what the pattern looks like that would cause that. Are you using any custom __getattr__ logic outside of what is done via lazy-loader? It could be that nested attach_stubs could cause this issue, but I don't immediately see the problem. It might be some interaction with the attatch_stub file.

@skoudoro
Copy link
Author

Thank you for the explanation @Erotemic. At some point I need to create a minimal example to understand in details what is going on.

Are you using any custom getattr logic outside of what is done via lazy-loader?

No

@drammock
Copy link
Member

@skoudoro MNE-Python hit a lot of circular imports in EAGER_IMPORT mode on CIs, after adopting lazy_loader. Sounds like a similar problem to what you faced. See mne-tools/mne-python#11838 (comment) We were able to fix it through some light-to-moderate refactoring of where some classes were defined, IIRC.

@Erotemic
Copy link

@drammock Your comment implies this wasn't a problem before using lazy-loader? Using lazy-loader will allow the user to avoid circular imports in some cases, but IIRC the eager mode should be the same as if you just used explicit import statements everywhere. If that isn't the case, then I'm looking for a MWE that demonstrates it so I can think about if there is a way to improve the eager mechanism.

@drammock
Copy link
Member

Your comment implies this wasn't a problem before using lazy-loader?

Sorry, it was a long time ago and I'm not the one who personally debugged the circular import issues, so I don't recall. @larsoner might remember.

@larsoner
Copy link

IIRC 1) we had no circular import issues, then as part of adding lazy loader we 2) un-nested some imports, then 3) EAGER_IMPORT had circular import issues. So yes I think it wasn't a lazy loader issue so much as an issue with our import scheme (i.e., if we removed lazy_loader and just used explicit imports, the same problem would have happened).

@Erotemic
Copy link

That makes sense. Part of the reason I wanted to include EAGER_IMPORT is to make those circular imports easier to find (as well as checking importing everything in the package works), but it does let you develop without needing to take circular imports into account, so if you expected it to just work, then that might be confusing.

This could be improved with better documentation, or perhaps if an import error is detected in an eager import, perhaps it could inform the user that a circular import could be the problem and perhaps link to some docs.

@stefanv
Copy link
Member

stefanv commented Feb 24, 2025

IIRC, lazy loader is also stricter i.t.o. circular imports than Python, which by default tolerates some circularity and figures out a way around it.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

5 participants