"Cannot determine type" on innocent but import-cycled code

[gnprice]

The following code is perfectly well-typed, but -- depending on the order files are passed to mypy -- gives a cryptic error message:

$ head *
==> part1.py <==
from part3 import part3_thing
class FirstThing: pass

==> part2.py <==
from part4 import part4_thing

import socket
Thing = socket.socket

==> part3.py <==
from part2 import Thing
def part3_thing(x, *y): pass
_thing = Thing()

==> part4.py <==
MYPY = False
if MYPY:
    from part1 import FirstThing
part4_thing = 1

On running mypy part[1234].py, we get the result

part3.py:3: error: Cannot determine type of 'Thing'

The import cycle seems to be essential. The result is also sensitive to order -- for example, mypy part{1,3,2,4}.py gives no error.

The repro can probably be reduced further; for example, I suspect there's a way to see it with three files, perhaps just two.

The code above obviously looks somewhat artificial -- it's stripped down from an example in a large internal codebase at Dropbox. We've seen several different people independently run into errors that look much like this in different parts of the same codebase, which has a number of import cycles, and I suspect the same thing is likely to happen in other large import-cycled codebases elsewhere. It'd be valuable to understand just what's going on here.

At a minimum, we should give a clear error message -- this one is pretty unhelpful for the user. Better yet we should fix the issue :), but import cycles have been tricky for us before so that might be difficult. If we can't immediately fix it, though, it'd be helpful even just to know what the obstacle is in this case.

[JukkaL]

We get the error if part2 is type checked later than part3. The order of processing in the cycle depends on the order of arguments on the command line, which is a little unfortunate.

Here are some ideas for improving this:

• We could flag the import in part4 as low priority (when sorting the import cycle), since it's in a if MYPY block and won't get evaluated at runtime. This would likely make the order of processing modules more deterministic and would fix the error message, though this might not work for some more complex examples.
• It should be possible to say from socket import socket as Thing to work around the issue but this currently results in a different error (Module has no attribute 'Thing' in part3). This actually looks like a separate problem -- I'll file another issue.
• Generate a message such as part2.py:4: error: Need type annotation for variable to make it easier to work around the issue by adding a type annotation.
• Infer types for simple aliases such as Thing = socket.socket earlier, during semantic analysis. We already infer types for things like x = 0 in semantic analysis and this example would likely not be difficult to support as well. This would fix the error message.

[tonygrue-dbx]

i hit what I believe to be this bug twice recently. annotating the variable didn't help. i played around with the order to try and resolve it, and it took some guessing and a few tries but I figured it out. however as we type more and more files, i'm worried we might become unable to work around this (other than removing cycles like these, which we're trying to add mypy to aid with =D).

[euresti]

Just got a much smaller repro, 3 files!

interface.py:

from __future__ import absolute_import

from reporting import global_id
from i18n import trans

class Account(object):
    def __init__(self):
        # type: () -> None
        self.account_id = None  # type: int

    def do_stuff(self):
        # type: () -> None
        # The following are correctly flagged as type errors.
        global_id()
        trans('OK', 'calling with bad args', 'but it passes anyway')

reporting.py:

from __future__ import absolute_import

# This causes a problem:
MYPY = False
if MYPY:
    # Avoid the circular import.
    from interface import Account

def get_id(account=None):
    # type: (Account) -> int
    if account:
        return account.account_id
    return 7

# The following causes mypy to fail with:
# i18n.py:11: error: Cannot determine type of 'global_id'
global_id = get_id()

# Same as above:
# import os.path
# global_id = os.path.join('foo', 'bar')

# But the following fixes them.

# The following works.
# global_id = 7

# The following works:  Even though now it's a function.
# def global_id():
#     # type: () -> int
#     return do_stuff()

# Also changing the import to `import interface` makes it go away, (after changing get_id)

i18n.py

from __future__ import absolute_import

from reporting import global_id

def trans(msgid):
    # type: (str) -> str
    return None

def _helper(syslang, possibilities, _country_code=None):
    # type: (str, List[str], str) -> str
    print(global_id)  # This causes failures.

This only happens when the files are checked in the following orders:
('interface.py', 'reporting.py', 'i18n.py') failed
('reporting.py', 'interface.py', 'i18n.py') failed
('reporting.py', 'i18n.py', 'interface.py') failed

But works when checked in this order:
('interface.py', 'i18n.py', 'reporting.py') passed
('i18n.py', 'interface.py', 'reporting.py') passed
('i18n.py', 'reporting.py', 'interface.py') passed

It fails both with --py2 and without it.

Also some of the issues go away if I change my imports to import x rather than from x import y

EDITED: Removed issues caused by not typing do_stuff.

[gvanrossum]

Also some of the issues go away if I change my imports to import x rather than from x import y

That's because we try to break the import cycles by prioritizing dependencies, and from x import y gets a higher priority than import x. Imports inside a function (of either form) have an even lower priority.

We're actually hoping that a fair fraction if cycle-related problems can be made to go away by assigning more subtle priorities, e.g. an import used for a base class may get an even higher priority.

[gvanrossum]

David suggested a fix for many occurrences: instead of writing

MYPY = False
if MYPY:
    from mod import C

(and then using C in annotations) you should write

MYPY = False
if MYPY:
    import mod

(and then using mod.C instead of plain C in annotations).

[euresti]

Ack. Just noticed that Any shows up because def do_stuff(self): doesn't have a type annotation. So never mind that part. I edited the post to remove that.

[gvanrossum]

This really is the same issue as #481, and I'm going to close this one as a duplicate (even though I like the example from the first comment).

[gvanrossum]

Sorry, reopening because this example must be fixed by improving the import order; it cannot be fixed by deferring the processing of functions.

[gvanrossum]

This will be fixed by #2167.

(Note that the two examples are almost but not quite the same. The way Thing is derived from socket.socket in part2.py differs, and the resulting error message also differs. But both are fixed by the indicated PR.)

[gvanrossum]

Fixed by #2167.