Skip to content

MAINT: delay initialization of getlimits (circular imports) #12064

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

Merged
merged 2 commits into from
Nov 1, 2018

Conversation

ahaldane
Copy link
Member

During module initialization, getlimits.py previously called numpy code before all of numpy was loaded,
which often causes circular import issues. This PR delays getlimits init.

This partly reverts and also continues PR #9113. Here, I made the whole of getlimits delay initialization. It's not good for any numpy code at all to be run before numpy is fully loaded.

Fixes #12063

@charris
Copy link
Member

charris commented Oct 1, 2018

This works. I wonder if there is another way that keeps things together that hasn't occurred to us. @eric-wieser Thoughts? This sort of thing isn't unique, ISTR that there are other places where import order is important.

@charris
Copy link
Member

charris commented Oct 1, 2018

Go to close/open this to experiment with azure.

@charris
Copy link
Member

charris commented Oct 10, 2018

I'm not particularly happy with this, it seems messy :( But I'll put it in if noone can suggest a better approach. I assume that the problem cannot be fixed by changing import order? Could the package be moved?

getlimits previously called numpy code before all of numpy was loaded,
which often causes circular import issues. This delays getlimits init.

Fixes numpy#12063
@ahaldane ahaldane force-pushed the delay_getlimit_init branch from 589ef4c to 699dfee Compare October 10, 2018 03:47
@ahaldane
Copy link
Member Author

Can modules in core be moved? Currently people can import it as import numpy.core.getlimits, and the fact that there are no underscores suggests it is public and can't be moved.

I agree it's ugly. I just added a commit trying to make it more palatable by removing all the global variable declarations and underscores, other ideas welcome. I can drop the commit if you like the old commit better.

@eric-wieser
Copy link
Member

@ahaldane: See my proposal at #11924 regarding moving modules in core.

@charris
Copy link
Member

charris commented Oct 31, 2018

Thinking about this, it does seem to me that execution of getlimits should be part of the module initiation, so I guess the question comes down to the best way to organize that.

@charris charris merged commit 8a5cd2c into numpy:master Nov 1, 2018
@charris
Copy link
Member

charris commented Nov 1, 2018

Thanks Allan, let's get this in. I wonder if we should pull most of this into the init file itself? I think we could drop the fallback code and just raise an error for unrecognized types.

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

Successfully merging this pull request may close these issues.

3 participants