Adding relative import support (#60)

[mason-bially]

This provides #60.

• Changing the all_imported_modules_in_file(self, file) projection in build which gathers imports from file ASTs root nodes.
• Adding support for the relative import syntax in noderepr.py and output.py.
• Adding a relative counter int to the AST nodes and fixing up treetransform.py.
• Changing the parse_import_from(self) function to parse relative imports.

[JukkaL]

Thanks for the pull request! I should be able to review it during this weekend.

[JukkaL]

Ah I wasn't quite able to make it... been finishing the final (hopefully!) corrections to my PhD dissertation and recovering from a cold. I should have more time the next week.

[JukkaL]

I played around with it a bit. It seems that full names of references via modules imported using a relative import are not normalized during semantic analysis. Here's a failing test case that I wrote (it would go to mypy/test/data/semanal-modules.test):

[case testRelativeImport]
import m.x
m.x.z.y
[file m/__init__.py]
[file m/x.py]
from . import z
[file m/z.py]
y = 1
[out]
MypyFile:1(
  Import:1(m.x : m.x)
  ExpressionStmt:2(
    MemberExpr:2(
      MemberExpr:2(
        MemberExpr:2(
          NameExpr(m)
          x [m.x])
        z [m.z])            # <== this is currently m.x.z, which is wrong
      y [m.z.y])))         # <== this has currently no full name at all
MypyFile:1(
  tmp/m/x.py
  ImportFrom:1(., [z : z]))
MypyFile:1(
  tmp/m/z.py
  AssignmentStmt:1(
    NameExpr(y* [m.z.y])
    IntExpr(1)))

It works as expected if I replace the relative import with an absolute one.

[JukkaL]

Here's another test case that fails:

[case testRelativeImport3]
import m
m.x.y
[file m/__init__.py]
from . import x
[file m/x.py]
y = 1
[out]
TODO: add expected output

It fails with an exception:

Traceback (most recent call last):
  File "mypy/test/data.py", line 136, in run
    self.perform(self)
  File "/home/jukka/project/mypy/mypy/test/testsemanal.py", line 52, in test_semanal
    alt_lib_path=test_temp_dir)
  File "mypy/build.py", line 166, in build
    result = manager.process(UnprocessedFile(info, program_text))
  File "mypy/build.py", line 352, in process
    '{} still unprocessed in state {}'.format(s.path, s.state()))
AssertionError: main still unprocessed in state 2

[mason-bially]

Ah, I forgot to fix the semantic analyzer (semanal.py:597) however there is a problem, the only variable I have access to for the current module id (cur_mod_id) does not provide enough information about whether the current module is an __init__ or not. To see why this is important consider:

# file: bar.py
## `semanal.py:598`; self.cur_mod_id; => "bar"
from . import foo


# file: baz/__init__.py
## `semanal.py:598`; self.cur_mod_id; => "baz"
from .. import foo

## Does not work in file `baz/__init__.py`, instead searches for `baz/foo.py`:
"""
from . import foo
"""
## => ImportError: cannot import name 'foo'

In my fix for build.py I used a 'hack' to check if the file in question was an __init__.py or not because the code in question had access to os.path, and was using the file anyway. However I'm not sure what the best way forward here is. It doesn't seem like the MypyFile node is stored anywhere useful, but even that doesn't provide the information I need (the name property doesn't seem to provide anything useful).

Perhaps adding a function to MypyFile along the lines of is_init(), which can be computed if the node in the future gains the information for making the determination from other arguments, but for now is set with the initalizer? One example here is to give the node initializer the full name ("bar.__init__" and let the (now rather strangely named) fullname() function return "bar" (slice off the "__init__")) and have is_init() check the given full name for the "__init__" ending.

This is a much bigger change to the node classes than my rather minor changes to the ImportFrom and ImportAll node which are sparsely used anyway, hence why I ask.

[JukkaL]

Could you use the path attribute of MypyFile? Maybe check that os.path.basename(mypy_file.path) == '__init__.py'. (Note that I haven't checked if this would work.)

Adding a method such as is_package_init_file() to MypyFile seems reasonable.

[mason-bially]

That sounds good (didn't realize it had a path attribute), the problem now is that currently on visitation of a MypyFile it saves the fullname() of the given MypyFile instead of the MypyFile node itself. This seems like a code smell because now self.cur_mod_id == self.cur_mod_node.fullname(). But I can do that if that's liveable. I could also change every instance of self.cur_mod_id 😧.

[mason-bially]

Of course that would also allow other common functionality to be refactored onto the node class, which may reduce code/complexity.

[JukkaL]

I don't think it's an issue to have both self.cur_mod_id and self.cur_mod_node, as they would be initialized in the same location. Having an alias for current module id simplifies code a bit.

[JukkaL]

@mason-bially Just checking if you are still working on this, as this would be a great feature to have? :-)

[mason-bially]

@JukkaL Sometime this weekend I should have the finished patch for review again.

[JukkaL]

@mason-bially Good!

[JukkaL]

@mason-bially Are you still working on this? It would be great to have relative import support, and I might work on this myself at some point. I'm cleaning up pull requests and wondering if this should be closed for now.

[mason-bially]

So this is working. I have a few more tests to add, with specific attention to __init__.py files. I am having a little trouble writing the tests. For example this test:

[case testRelativeImport3]
import m.t
m.zy
m.xy
m.t.y
[file m/__init__.py]
from .x import *
from .z import *
[file m/x.py]
from .x import zy as xy
[file m/z.py]
zy = 3
[file m/t/__init__.py]
from .b import *
[file m/t/b.py]
from .. import xy as y
[out]
MypyFile:1(
  Import:1(m.t : m.t)
  ExpressionStmt:2(
    MemberExpr:2(
      NameExpr(m [m])
      zy [m.zy]))
  ExpressionStmt:3(
    MemberExpr:3(
      NameExpr(m [m])
      xy [m.zy]))
  ExpressionStmt:4(
    MemberExpr:4(
      MemberExpr:4(
        NameExpr(m [m])
        t [m.t])
      y [m.zy])))
# etc...

Fails. It appears that placing imports in inits (or perhaps any code) causes them to fail with an assertion error exactly like:

Traceback (most recent call last):
  File "H:\ProgramFiles\mypy\mypy\test\data.py", line 136, in run
    self.perform(self)
  File "H:\ProgramFiles\mypy\mypy\test\testtransform.py", line 46, in test_transform
    alt_lib_path=test_temp_dir)
  File "H:\ProgramFiles\mypy\mypy\build.py", line 165, in build
    result = manager.process(UnprocessedFile(info, program_text))
  File "H:\ProgramFiles\mypy\mypy\build.py", line 353, in process
    '{} still unprocessed in state {}'.format(s.path, s.state()))
AssertionError: main still unprocessed in state 2

This is where I was stuck a couple of months ago. I wasn't able to figure out why it was failing.

[mason-bially]

I guess we had this problem previously as well. I don't quite understand what's going wrong there. What does that error mean? Any ideas why imports in __init__.py files might cause that error?

[mason-bially]

I think I have a better understanding of what is happening. The relative imports in __init__.py files are somehow preventing the semantic analyzer from finishing analysis of all of the files. And I have no idea why that would be.

[JukkaL]

I can look into this. The code that deals with module dependencies and different build states is pretty hairy.

[JukkaL]

There was an unrelated bug in build.py. I fixed it and things started working :-D