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

Discovery strategy is still mysterious w/PEP 420 namespace packages #6989

Closed
warsaw opened this issue Jun 13, 2019 · 9 comments · Fixed by #7219
Closed

Discovery strategy is still mysterious w/PEP 420 namespace packages #6989

warsaw opened this issue Jun 13, 2019 · 9 comments · Fixed by #7219
Labels

Comments

@warsaw
Copy link
Member

warsaw commented Jun 13, 2019

I'm having trouble understanding some of the subtleties of how mypy 0.701 (w/Python 3.7) discovers files to check. Here is a sample directory tree:

.
└── lipy-foo
    ├── setup.cfg
    └── src
        └── linkedin
            └── foo
                ├── __init__.py
                └── bar.py

linkedin is a PEP 420 namespace package and linkedin/foo/__init__.py is empty. bar.py contains:

def bar(a: int, b: int) -> str:
    return a + b

When I cd into the lipy-foo directory containing the setup.cfg, and try to run mypy, it can't find any .py files to check:

% ls
setup.cfg	src/
% mypy src --namespace-packages -v
There are no .py[i] files in directory 'src'

Clearly mypy doesn't recurse into the src directory to find the linkedin namespace package. Other options that don't work include:

% mypy -p linkedin --namespace-packages
Can't find package 'linkedin'
% MYPYPATH=src mypy -p linkedin --namespace-packages
Can't find package 'linkedin'

What does work:

% mypy src/linkedin --namespace-packages
src/linkedin/foo/bar.py:2: error: Incompatible return value type (got "int", expected "str")

Also, if I add an empty src/linkedin/__init__.py file, discovery does work as expected:

% tree
.
├── setup.cfg
└── src
    └── linkedin
        ├── __init__.py
        └── foo
            ├── __init__.py
            └── bar.py
% mypy src --namespace-packages --no-site-packages
src/linkedin/foo/bar.py:2: error: Incompatible return value type (got "int", expected "str")

So PEP 420 style namespace packages do seem to influence the behavior here.

Since we may have additional directories and namespace packages under src in other repositories, I'd prefer not to have to invoke mypy with src/linkedin, but there doesn't seem to be any other option.

Maybe I'm doing something wrong, so please tell me if this is pebkac, or is unsupported. Should mypy be able to discover the linkedin namespace package using src-style package layouts with PEP 420 namespaces?

@ilevkivskyi
Copy link
Member

I think @gvanrossum is the best person to ask about this.

@gvanrossum
Copy link
Member

This is totally mypy's fault. Namespace packages are only discovered during import following, not during initial file discovery from the command line. I recommend dropping an empty __init__.pyi (note: .pyi) in the namespace package.

@warsaw
Copy link
Member Author

warsaw commented Jun 14, 2019

Thanks for the follow up Guido. Adding an empty placeholder file isn't really an option for us, for reasons not worth going into here.

Namespace packages are only discovered during import following, not during initial file discovery from the command line.

Would it make sense, when the -p option is given, to do the import and use the resulting module's __spec__.submodule_search_locations to find the directory to start scanning? Something like the following:

mypy -p linkedin --namespace-packages would be morally equivalent to:

$ python3 -c "import linkedin; print(linkedin.__spec__.submodule_search_locations)"
_NamespacePath(['/path/to/lipy-foo/src/linkedin'])

Then you'd just iterate over the elements?

Alternatively, if a src directory is given, just continue to recursively traverse until you find a concrete package and .py files to start scanning.

I'm sure both of these have subtitles that make it tricky. If we must, we'll have to auto-discover the subdirectories ourselves and pass them along on the command line.

@ilevkivskyi
Copy link
Member

Btw, it looks like this is related to #6385

@gvanrossum
Copy link
Member

I would rather not have mypy "do the import" -- that could run code which might have undesirable side effects, or it could just fail mysteriously. (In general we're pretty adamant that mypy shouldn't ever run your code in order to check it.) Also mypy won't know that the argument refers to a namespace package until it's too late.

I think the approach you mentioned that "does work", mypy src/linkedin --namespace-packages, doesn't actually really work, because it ends up thinking that the fullly-qualified name of bar.py is foo.bar, whereas it should really be linkedin.foo.bar. This distinction becomes important when other files are being checked.

I suppose the most surprising part is that MYPYPATH=src mypy -p linkedin --namespace-package doesn't work. Possibly we could make this work with some tweaks to the implementation of the -p flag in mypy/main.py. (Although it would be slightly inconsistent, since mypy's package discovery still wouldn't work if you were to write mypy src/linkedin/foo/bar.py -- that would still think the full name is foo.bar.)

Would you be interested in giving this a try?

@warsaw
Copy link
Member Author

warsaw commented Jun 26, 2019

Would you be interested in giving this a try?

Yes! I think it would be fine if the second invocation form was a little inconsistent.

Here's how I'm thinking about it. Let's say I do this:

% touch src/linkedin/__init__.pyi
% MYPYPATH=src mypy -p linkedin --namespace-packages --no-site-packages
src/linkedin/foo/bar.py:2: error: Incompatible return value type (got "int", expected "str")

That makes sense to me; I want to check the linkedin namespace anchored underneath src. What bothers me is that I have to create a dummy __init__.pyi file to make this work, and that this doesn't work the same way:

% rm -f src/linkedin/__init__.pyi 
% MYPYPATH=src mypy -p linkedin --namespace-packages --no-site-packages
Can't find package 'linkedin'

Is that a reasonable thing to do?

@gvanrossum
Copy link
Member

I think we all agree that that should work. I was hoping that you'd try to come up with a PR for mypy to make it work that way, since the whole mypy team is basically super distracted by other priorities at the moment...

@warsaw
Copy link
Member Author

warsaw commented Jun 26, 2019

Fair enough! I'll give it a go, although it likely won't be until after the week of July 4th.

@warsaw
Copy link
Member Author

warsaw commented Jul 17, 2019

See the above PR. I've done internal testing and it works well for us.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants