Skip to content
This repository has been archived by the owner on Apr 14, 2022. It is now read-only.

Implement declaration of submodules in module members #1565

Merged
merged 24 commits into from
Sep 26, 2019

Conversation

MikhailArkhipov
Copy link

@MikhailArkhipov MikhailArkhipov commented Sep 18, 2019

Fixes #1098
Fixes #1301
Fixes #1395

Add submodules to module members so they show up in completions on the module name. Otherwise we show them in

image

but not in

import torch
torch.

MikhailArkhipov added 4 commits September 19, 2019 12:50
Declare submodule as member in case of import A.B inside A
@MikhailArkhipov
Copy link
Author

MikhailArkhipov commented Sep 19, 2019

image

image

image

Python 3.7, pip-installed 1.2

@MikhailArkhipov
Copy link
Author

image

image

@MikhailArkhipov MikhailArkhipov changed the title Add submodules to module members Implement declaration of submodules in module members Sep 19, 2019

await Services.GetService<IPythonAnalyzer>().WaitForCompleteAnalysisAsync();
var analysis = await doc.GetAnalysisAsync(Timeout.Infinite);
analysis.Should().HaveVariable("top").Which.Should().HaveMembers("sub1", "sub2", "sub3", "top");
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What should happen here in __init__.py itself, e.g.:

var top = rdt.GetDocument(TestData.GetTestSpecificUri("top", "__init__.py"));
var topAnalysis = await top.GetAnalysisAsync(-1);
analysis.Should().HaveVariable("sub2").And.NotHaveVariable("sub3");

Copy link
Contributor

@AlexanderSher AlexanderSher Sep 19, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Also, I would've check sub3 with something like this:

await TestData.CreateTestSpecificFileAsync(Path.Combine("top", "sub3", "__init__.py"), @"
import top.sub3.sub4
");

and then

var sub3 = rdt.GetDocument(TestData.GetTestSpecificUri("top", "sub3", "__init__.py"));
var sub3Analysis = await sub3.GetAnalysisAsync(-1);
analysis.Should().HaveVariable("sub4").And.NotHaveVariable("sub3");

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

analysis.Should().HaveVariable("sub2").And.NotHaveVariable("sub3"); - top actually has sub3 which is produced by import top.sub3.sub4, as tested in the console.

Copy link
Author

@MikhailArkhipov MikhailArkhipov Sep 19, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

And import top.sub3.sub4 does not seem to declare sub4

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I.e. in the

Python 3.7.2 (tags/v3.7.2:9a3ffc0492, Dec 23 2018, 23:09:28) [MSC v.1916 64 bit (AMD64)] on win32
>>> import top
>>> dir(top)
['__builtins__', '__cached__', '__doc__', '__file__', '__loader__', '__name__', '__package__', '__path__', '__spec__', 'sub1', 'sub2', 'sub3', 'top']
>>>

So basically

from top import sub1
import top.sub2
import top.sub3.sub4

yields members top, sub1, sub2 and sub3 but not sub4.

@jakebailey
Copy link
Member

Here's an interesting example where the top level module is able to essentially "add" members to child modules. Maybe we also need to be doing this via VariableModule.

# a/__init__.py
import a.b.c.d
class A:
    pass

# a/b/__init__.py
class B:
    pass

# a/b/c/__init__.py
class C:
    pass

# a/b/c/d/__init__.py
class D:
    pass

Running:

import a

def print_members(prefix, x):
    print(prefix, [m for m in dir(x) if not m.startswith("__")])

print_members("members of a:      ", a)
print_members("members of a.b:    ", a.b)
print_members("members of a.b.c:  ", a.b.c)
print_members("members of a.b.c.d:", a.b.c.d)

Gives:

members of a:       ['A', 'a', 'b']
members of a.b:     ['B', 'c']
members of a.b.c:   ['C', 'd']
members of a.b.c.d: ['D']`

@jakebailey
Copy link
Member

And unfortunately, this effect persists to imports of another form:

import a

def print_members(prefix, x):
    print(prefix, [m for m in dir(x) if not m.startswith("__")])

print_members("members of a:      ", a)
print_members("members of a.b:    ", a.b)
print_members("members of a.b.c:  ", a.b.c)
print_members("members of a.b.c.d:", a.b.c.d)

from a.b import c

print_members("members of c:      ", c)
members of a:       ['A', 'a', 'b']
members of a.b:     ['B', 'c']
members of a.b.c:   ['C', 'd']
members of a.b.c.d: ['D']
members of c:       ['C', 'd']

@jakebailey
Copy link
Member

jakebailey commented Sep 24, 2019

Using my example above, seeing this weird completion (note the a.b.c.d in addition to the "real" option d):

image

Also, from a.b import c resolves the variable c in a.b, but c should be the module a.b.c instead. a.b.c is a module as expected.

Copy link
Member

@jakebailey jakebailey left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I've tested this with a bunch of different projects. Seems to work fine. We can merge it and see.

@MikhailArkhipov MikhailArkhipov merged commit 7671f2c into microsoft:master Sep 26, 2019
@MikhailArkhipov MikhailArkhipov deleted the 1301 branch September 26, 2019 00:07
jakebailey pushed a commit to jakebailey/python-language-server that referenced this pull request Nov 1, 2019
* Add submodules to module members

* Add test

* Test updates

* Wait for complete analysis

* Different way of filtering

* Formatting

* Remove unused

* Move filtering to completions
Declare submodule as member in case of import A.B inside A

* Add another test

* using

* De-duplicate

* Remove unused

* Revert to initial GetMemberNames/GetMember on module

* Prefer submodule over variable

* Use child modules instead

* Extra checks

* Fix child module naming

* Prefer submodules to variables.

* Prefer submodules to variables in import star
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
4 participants