Skip to content

MAINT: Move set_module from numpy.core to numpy._utils#22619

Merged
mattip merged 4 commits intonumpy:mainfrom
seberg:move-set_module
Nov 29, 2022
Merged

MAINT: Move set_module from numpy.core to numpy._utils#22619
mattip merged 4 commits intonumpy:mainfrom
seberg:move-set_module

Conversation

@seberg
Copy link
Copy Markdown
Member

@seberg seberg commented Nov 18, 2022

Note sure if others agree, this creates a numpy._utils module, because numpy.core already imports the C stuff.
The reason was that I wanted to use set_module when moving exceptions (since they currently have that). But set_module being defined in overrides needs C to be imported.

So I thought, maybe we actually need a place to put things that don't need depend on anything else to avoid circular imports...

If nobody else likes the idea, I am happy to close and just use object.__module__ = "numpy", or put it elsewhere (but I didn't have a good idea for that).

@seberg
Copy link
Copy Markdown
Member Author

seberg commented Nov 18, 2022

Note that I have not changed imports that are using from ...overrides import something, set_module, since they need the rest anyway.

@mattip
Copy link
Copy Markdown
Member

mattip commented Nov 24, 2022

Maybe make this numpy.lib._utils?

@seberg
Copy link
Copy Markdown
Member Author

seberg commented Nov 24, 2022

Yeah, I wanted to put it somewhere deeper, and I don't quite love this either (so happy for other ideas even duplicating the helper).
The problem is, I also want this to be available before any C import happens... lib pulls in the public submodules, which pull in all of NumPy (circular imports).

@mattip mattip changed the title MAINT: Move set_module to numpy.core to use without C import MAINT: Move set_module from numpy.core to numpy._utils Nov 25, 2022
@mattip
Copy link
Copy Markdown
Member

mattip commented Nov 25, 2022

I guess numpy._utils makes sense. Perhaps we should move numpy/compat/_inspect.py there as well.

@mattip
Copy link
Copy Markdown
Member

mattip commented Nov 25, 2022

Ahh, could we have a numpy._utils (maybe a different name?) directory, and in that have a set_module.py file? Then in the __init__.py it could say something about "A place for code that does not import from the rest of NumPy, to reduce circular dependencies"

@seberg
Copy link
Copy Markdown
Member Author

seberg commented Nov 25, 2022

Yeah, a submodule/folder make sense, how about being super clear, like _pydefs?

_globals and _inspect could go there as well, then, yeah. Maybe more? I am not sure I love it, but a place to reduce circular imports seems like it likely makes sense anyway.

@mattip
Copy link
Copy Markdown
Member

mattip commented Nov 25, 2022

how about being super clear, like _pydefs

I am bad with names. We are looking for a place to put things with no dependencies. Do we want to definitely restrict ourselves to pure-python modules or can we also put some simple no-dependency c-extension module in the future? I think I prefer something in the direction of utils or basic

Note that unfortunately, compat does expose _inspect as well,
so the import remains (just the definition place moves).
@seberg
Copy link
Copy Markdown
Member Author

seberg commented Nov 25, 2022

Hmmm, yeah, i don't the name _pydefs either anyway. _utils seems fine to me, it is at least a place where you would look for these things?

Created a module, left set_module in the __init__ for the moment. Moved _inspect and _pep440. _inspect is actuall imported to np.compat, but maybe it makes sense to move it anyway?

"""
from . import _inspect

from .._utils import _inspect
Copy link
Copy Markdown
Member

@mattip mattip Nov 27, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is this line required?

Ahh, I see, yes it is.

Copy link
Copy Markdown
Member

@mattip mattip left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM.

@mattip
Copy link
Copy Markdown
Member

mattip commented Nov 28, 2022

I will put this in soon if there are no further comments

@mattip mattip merged commit b0b7a06 into numpy:main Nov 29, 2022
@mattip
Copy link
Copy Markdown
Member

mattip commented Nov 29, 2022

Thanks @seberg

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.

2 participants