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

Incorrect type annotations cause AttributeError on import #851

Closed
pranavrajpal opened this issue May 13, 2021 · 2 comments · Fixed by python/mypy#10550
Closed

Incorrect type annotations cause AttributeError on import #851

pranavrajpal opened this issue May 13, 2021 · 2 comments · Fixed by python/mypy#10550
Labels
bug python compat Mypyc doesn't match CPython or documented semantics.

Comments

@pranavrajpal
Copy link

With the following code (using the Pillow library):

from PIL import Image

def test(im: Image):
    print(im.mode)

def run() -> None:
    im = Image.open('test.png')
    test(im)

and the following config file:

[mypy-PIL.*]
ignore_missing_imports = True

mypy shows no errors, but compiling the code with mypyc and trying to run it produces the following traceback:

Traceback (most recent call last):
  File "<string>", line 1, in <module>
  File "bug.py", line 1, in <module>
    from PIL import Image
AttributeError: module 'PIL' has no attribute 'Image'

The problem is that the type annotations in the original code are incorrect, because PIL.Image is a module, and PIL.Image.open is a function that returns a separate PIL.Image.Image object, so the correct type annotations are:

from PIL import Image
from PIL.Image import Image as ImageType

def test(im: ImageType):
    print(im.mode)

def run() -> None:
    im = Image.open('test.png')
    test(im)

which fixes the mypyc error.


There are a couple of possible solutions that I can think of to this:

  1. Get mypy to realize the error in the original code, even when ignore_missing_imports is set, by noticing that a class is being passed in when a module is expected.
  2. Same as (1), but only emit an error if compiling with mypyc.
  3. Ignore any type annotations from untyped libraries when compiling with mypyc.
  4. Insert code into the compiled C extension to check if this happens and print/raise with a nicer error message.

I'd like to work on this, but I don't really know which (if any) of the solutions I mentioned would be the best way to fix this.

@JelleZijlstra
Copy link
Collaborator

This looks like mypyc miscompiles from A import B as essentially import A; B = A.B.

Here's a snippet from the generated C code:

CPyL4: ;
    cpy_r_r8 = CPyStatic_unicode_1; /* 'PIL' */
    cpy_r_r9 = PyImport_Import(cpy_r_r8);
    if (unlikely(cpy_r_r9 == NULL)) {
        CPy_AddTraceback("pilly.py", "<module>", 1, CPyStatic_globals);
        goto CPyL9;
    } else
        goto CPyL5;
CPyL5: ;
    CPyModule_PIL = cpy_r_r9;
    CPy_INCREF(CPyModule_PIL);
    CPy_DecRef(cpy_r_r9);
    goto CPyL6;
CPyL6: ;
    cpy_r_r10 = CPyModule_PIL;
    cpy_r_r11 = CPyStatic_globals;
    cpy_r_r12 = CPyStatic_unicode_2; /* 'Image' */
    cpy_r_r13 = CPyObject_GetAttr(cpy_r_r10, cpy_r_r12);

The cpy_r_r9 line does essentially import PIL, and the cpy_r_r13 lines does getattr(PIL, "Image"). But that is wrong, because from A import B may end up importing A.B as a module.

This still reproduces if the module is just from PIL import Image, so it's not related to the incorrect type annotation in your code.

The solution would be to make the C code mypyc generates follow the runtime semantics of from ... import more closely.

@pranavrajpal
Copy link
Author

What you're saying does make sense. I think the reason that the second python script happened to work correctly is because, looking at the generated C code:

CPyL4: ;
    cpy_r_r8 = CPyStatics[9]; /* 'PIL' */
    cpy_r_r9 = PyImport_Import(cpy_r_r8);
    if (unlikely(cpy_r_r9 == NULL)) {
        CPy_AddTraceback("fixed_bug.py", "<module>", 1, CPyStatic_globals);
        goto CPyL17;
    }
CPyL5: ;
    CPyModule_PIL = cpy_r_r9;
    CPy_INCREF(CPyModule_PIL);
    CPy_DecRef(cpy_r_r9);
CPyL6: ;
    cpy_r_r10 = CPyModule_PIL;
    cpy_r_r11 = CPyStatic_globals;
    cpy_r_r12 = CPyModule_PIL___Image;
    cpy_r_r13 = (PyObject *)&_Py_NoneStruct;
    cpy_r_r14 = cpy_r_r12 != cpy_r_r13;
    if (cpy_r_r14) goto CPyL9;
CPyL7: ;
    cpy_r_r15 = CPyStatics[10]; /* 'PIL.Image' */
    cpy_r_r16 = PyImport_Import(cpy_r_r15);
    if (unlikely(cpy_r_r16 == NULL)) {
        CPy_AddTraceback("fixed_bug.py", "<module>", 1, CPyStatic_globals);
        goto CPyL17;
    }
CPyL8: ;
    CPyModule_PIL___Image = cpy_r_r16;
    CPy_INCREF(CPyModule_PIL___Image);
    CPy_DecRef(cpy_r_r16);
CPyL9: ;
    cpy_r_r17 = CPyStatics[5]; /* 'Image' */
    cpy_r_r18 = CPyObject_GetAttr(cpy_r_r10, cpy_r_r17);
    if (unlikely(cpy_r_r18 == NULL)) {
        CPy_AddTraceback("fixed_bug.py", "<module>", 1, CPyStatic_globals);
        goto CPyL17;
    }

It appears to be doing the equivalent of:

import PIL
import PIL.Image
Image = getattr(PIL, "Image")

which happens to do the same thing as the original version by coincidence.

I'll take a look at this and see what I find.

@JukkaL JukkaL added bug python compat Mypyc doesn't match CPython or documented semantics. labels May 24, 2021
msullivan pushed a commit to python/mypy that referenced this issue Jun 9, 2021
…10550)

Fixes mypyc/mypyc#851

This fixes a bug where code compiled with mypyc would crash on from imports (from x import y) if:
 * y is a module
 * mypy doesn't know that y is a module (due to an ignore_missing_imports configuration option or something else)

The bug was caused by using getattr to import modules (i.e. y = getattr(x, 'y')) and changing this to import x.y as y when it can determine that y is a module. This doesn't work when we don't know that y is a module.

I changed the from import handling to use something similar to the method shown in the __import__ docs. I also removed the special casing of from imports for modules (from x import y where y is a module) mentioned earlier, because these changes make that special casing unnecessary.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug python compat Mypyc doesn't match CPython or documented semantics.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants