Skip to content
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

BUG: import full module path in npy_load_module #8148

Merged
merged 1 commit into from Oct 13, 2016

Conversation

jjhelmus
Copy link
Contributor

Use the full module path when importing importlib.machinery for use in the
npy_load_module function. Just importing importlib is not sufficient in certain
cases, for example Python 3.4.

closes #8147

Use the full module path when importing importlib.machinery for use in the
npy_load_module function. Just importing importlib is not sufficient in certain
cases, for example Python 3.4.

closes numpy#8147
@njsmith
Copy link
Member

njsmith commented Oct 12, 2016

Change looks good but can we get a test?

@jjhelmus
Copy link
Contributor Author

Was thinking about the following as a test:

def test_npy_load_module_py34():
    with tempdir(prefix="numpy_test_compat_") as folder:
        filename = join(folder, 'example.py')
        with open(filename, 'w') as f:
            f.write("magic_number = 42")
        example = npy_load_module('example', filename)
        assert_(example.magic_number == 42)

The issue is that this passes with or without the proposed fix. Outside of the test suite the assert will fail without the fix but inside the test suite it always passes.

@jjhelmus
Copy link
Contributor Author

Fun, machinery is not in the importlib module namespace if imported alone, but it is after nose is imported:

Python 3.4.5 |Continuum Analytics, Inc.| (default, Jul  2 2016, 17:47:57)
[GCC 4.2.1 Compatible Apple LLVM 4.2 (clang-425.0.28)] on darwin
Type "help", "copyright", "credits" or "license" for more information.
>>> import importlib
>>> dir(importlib)
['_RELOADING', '__all__', '__builtins__', '__cached__', '__doc__', '__file__', '__import__', '__loader__', '__name__', '__package__', '__path__', '__spec__', '_bootstrap', '_imp', '_r_long', '_w_long', 'find_loader', 'import_module', 'invalidate_caches', 'reload', 'sys', 'types', 'warnings']
>>> import nose
>>> dir(importlib)
['_RELOADING', '__all__', '__builtins__', '__cached__', '__doc__', '__file__', '__import__', '__loader__', '__name__', '__package__', '__path__', '__spec__', '_bootstrap', '_imp', '_r_long', '_w_long', 'find_loader', 'import_module', 'invalidate_caches', 'machinery', 'reload', 'sys', 'types', 'util', 'warnings']

I'm at a loss on how write a test that changes from pass to fail with the proposed modification.

@njsmith
Copy link
Member

njsmith commented Oct 12, 2016

Ugh, wow.

Hail mary: subprocess.run([sys.executable, "-c", "import numpy; numpy.npy_load_module(..."])?

Or if it's not doable we can skip it.

@charris
Copy link
Member

charris commented Oct 13, 2016

@njsmith I'm going to put this in, figuring out how to test it looks to be more trouble than it is worth.

@charris
Copy link
Member

charris commented Oct 13, 2016

Thanks @jjhelmus .

@jakirkham
Copy link
Contributor

Would this be backported to 1.11?

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.

Configuration.add_subpackage fails on Python 3.4
5 participants