Python version-conditional import inside function does not respect test

This is OK at the top level:

import sys
if sys.version_info[0] >= 3:
    import configparser
else:
    import ConfigParser as configparser

This causes the error as noted in the comment:

import sys
def hooper(): # type: () -> None
    if sys.version_info[0] >= 3:
        import configparser # if mypy -2: Cannot find module named 'configparser'
    else:
        import ConfigParser as configparser # if mypy: Cannot find module named 'ConfigParser'

Interesting. It looks like other cases of sys.version_info checks (not involving import) work correctly, so it seems the sys.version_info logic is executed here (also verified in the debugger). So maybe the code for understanding alternative imports isn't triggered inside functions?

So maybe the code for understanding alternative imports isn't triggered inside functions?

Even typing.TYPE_CHECKING seems to be ignored inside a function:

#!/usr/bin/env python

from __future__ import print_function
from typing import TYPE_CHECKING
import sys

def coolfun(): # type: () -> None
    if sys.version_info[0] >= 3:
        print("i'm py3 in a function")
    else:
        if not TYPE_CHECKING:
            import ConfigParser as configparser # hooper.py:12: error: Cannot find module named 'ConfigParser'
        print("i'm py2 in a function")

if __name__ == '__main__':
    coolfun()

Yeah, it seems that inside functions the is_unreachable flag is not respected by the import-handling code. I'll have to delve in deeper to understand why that is -- if you replace the alternative imports with conflicting assignments you'll see that the type checker does honor reachability everywhere. And the import code respects it outside functions.

I debugged this a bit more, it's because FirstPass doesn't descend into functions. I don't think this is a very important use case -- it should be easy to work around by moving the conditional imports out of functions.

It's quite common for imports, conditional or otherwise, to be done within functions (to improve startup time, to not bloat the interpreter's footprint unless a module is needed, to prevent namespace pollution, etc.). It's also fairly surprising for it to not work as expected.

Do you feel up to submitting a PR? It's not a particularly complex part of mypy.

I've never looked at mypy's internals apart from some collections stuff, so if you already have an idea of how to fix it, that's less likely to break something than if I have to figure it out. If you're busy, I don't mind taking a look, but unless the fix is obvious, you could probably take care of it faster than I could.

But probably not sooner. :-) We have over 450 issues in the tracker and we have to prioritize.

Any tips on what needs to be changed and how? (I guess the where is FirstPass of mypy/semanal.py.)

I'm guessing the function and class visitors need to recurse into their bodies. And we'll need tests. (Does the same problem occur with conditional imports in classes? If not the class visitor might not need fixing. The visitors for some other statements show you how to recurse.)

To traverse function and class bodies, I see these (from NodeVisitor) are already present:

def visit_block(self, o: 'mypy.nodes.Block') -> T:
def visit_while_stmt(self, o: 'mypy.nodes.WhileStmt') -> T:
def visit_for_stmt(self, o: 'mypy.nodes.ForStmt') -> T:
def visit_if_stmt(self, o: 'mypy.nodes.IfStmt') -> T:
def visit_try_stmt(self, o: 'mypy.nodes.TryStmt') -> T:
def visit_with_stmt(self, o: 'mypy.nodes.WithStmt') -> T:

So I guess only the function and class visitors should need changes, and the import-related stuff as to whether it's a top-level import or not (maybe?)

I expect the import visitors to be fine, it's the function/class visitors
and the statement visitors that you noticed that need work. (visitors
containing "pass" are not sufficient, they have to do the work
themselves. There are plenty of examples.)

On Sun, Oct 16, 2016 at 4:51 PM, Mark Laws notifications@github.com wrote:

To traverse function and class bodies, I see these (from NodeVisitor) are
already present

def visit_block(self, o: 'mypy.nodes.Block') -> T: passdef visit_while_stmt(self, o: 'mypy.nodes.WhileStmt') -> T: passdef visit_for_stmt(self, o: 'mypy.nodes.ForStmt') -> T: passdef visit_if_stmt(self, o: 'mypy.nodes.IfStmt') -> T: passdef visit_try_stmt(self, o: 'mypy.nodes.TryStmt') -> T: passdef visit_with_stmt(self, o: 'mypy.nodes.WithStmt') -> T: pass

So I guess only the function and class visitors should need changes, and
the import-related stuff as to whether it's a top-level import or not
(maybe?)

—
You are receiving this because you commented.
Reply to this email directly, view it on GitHub
#2260 (comment), or mute
the thread
https://github.com/notifications/unsubscribe-auth/ACwrMnz69kzUPt1KcpaO-vMvBEgtl81Lks5q0riQgaJpZM4KYAxm
.

--Guido van Rossum (python.org/~guido)

Are you still working on a PR for this?

Yes, sorry--I got sidetracked but I should have some time to look at it today.

I have a branch that works with all my test files (and real-world files where I wanted this) and passes all current tests. Would you be able to write tests for it? I've made a PR: #2304

Navigation Menu

Repository navigation

Python version-conditional import inside function does not respect test #2260

Metadata

Assignees

Labels

Projects

Milestone

Relationships

Development

Python version-conditional import inside function does not respect test #2260

Description

Activity

gvanrossum commented on Oct 16, 2016

drvink commented on Oct 16, 2016

gvanrossum commented on Oct 16, 2016

gvanrossum commented on Oct 16, 2016

drvink commented on Oct 16, 2016

gvanrossum commented on Oct 16, 2016

drvink commented on Oct 16, 2016

gvanrossum commented on Oct 16, 2016

drvink commented on Oct 16, 2016

gvanrossum commented on Oct 16, 2016

drvink commented on Oct 16, 2016

gvanrossum commented on Oct 17, 2016

gvanrossum commented on Oct 20, 2016

drvink commented on Oct 21, 2016

drvink commented on Oct 23, 2016

Metadata

Metadata

Assignees

Labels

Projects

Milestone

Relationships

Development

Issue actions