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

Unresolved import if sys.path modified in __init__.py #6241

Closed
farleylai opened this issue Jun 19, 2019 · 11 comments
Closed

Unresolved import if sys.path modified in __init__.py #6241

farleylai opened this issue Jun 19, 2019 · 11 comments
Labels
area-linting bug Issue identified by VS Code Team member as probable bug

Comments

@farleylai
Copy link

farleylai commented Jun 19, 2019

For projects that include external repos, it would be handy to insert the relative dependency paths ONLY when importing the necessary module.

Here is an example project layout:

Project root
+-- external repo/
      +-- lib/
           +-- config  # defines Config class
      +-- utils/
+-- mypackageA
     +-- __init__.py
     +-- app.py
main.py

In main.py, when an app instance that depends on the Config.py module in the external repo is created, the sys.path must include external repo/lib at the time of importing app:

from mypackageA import app
app.App()

Instead of setting the PYTHONPATH statically, an alternative is to modify sys.path in mypackageA.__init__.py by inserting paths to external repo/lib for the app module to import Config. However, vscode shows unresolved import warnings in this use case.

@farleylai farleylai added triage-needed Needs assignment to the proper sub-team feature-request Request for new features or functionality labels Jun 19, 2019
@ghost ghost removed the triage-needed Needs assignment to the proper sub-team label Jun 20, 2019
@ericsnowcurrently
Copy link
Member

ericsnowcurrently commented Jun 24, 2019

@farleylai, thanks for letting us know about this. If I understood correctly, mypackageA.app.py has something like from config import Config in it. You want the import to work automatically when using the extension, where it currently doesn't. Is that right? Otherwise I'm not clear on how we can help.

To get what you want, the extension would have to automatically add external repo/lib to sys.path. However, there are 2 problems with that:

  1. running main.py from the normal commandline already doesn't work like you want and likewise if you tried to distribute this project
  2. how is the extension supported to know that it should add external repo/bin to PYTHONPATH?

As to the first one, this situation is not unique to the extension. I recommend using one of the existing approaches out there.

@ericsnowcurrently ericsnowcurrently added the info-needed Issue requires more information from poster label Jun 24, 2019
@farleylai
Copy link
Author

farleylai commented Jun 24, 2019

@farleylai, thanks for letting us know about this. If I understood correctly, mypackageA.app.py has something like from config import Config in it. You want the import to work automatically when using the extension, where it currently doesn't. Is that right? Otherwise I'm not clear on how we can help.

@ericsnowcurrently
Yes, that is what I mean the path is inserted only when importing the specific module.

To get what you want, the extension would have to automatically add external repo/lib to sys.path. However, there are 2 problems with that:

  1. running main.py from the normal commandline already doesn't work like you want and likewise if you tried to distribute this project

This alternative I mentioned is to insert the path in myPackageA.__init__.py like the following:

import sys
sys.path.insert(0, 'external repo/lib')

I am saying the programmer has done this such that whenever app.py is imported, the statement from myPackage import app should have the additional path inserted. In this way, the main.py works as expected but VSCode does not seem to evaluate myPackageA.__init__.py when editing.

  1. how is the extension supported to know that it should add external repo/bin to PYTHONPATH?

So my first intuition is to have the extension to evaluate myPackageA.__init__.py when editing main.py or app.py. Could that be possible?

As to the first one, this situation is not unique to the extension. I recommend using one of the existing approaches out there.

@ericsnowcurrently ericsnowcurrently removed the info-needed Issue requires more information from poster label Jun 25, 2019
@ericsnowcurrently
Copy link
Member

Thanks for clarifying, @farleylai. I understand now and was able to quickly reproduce the problem. Based on the information you've provided thus far I was able to fill in the blanks for file content, which I've included below.

Using Jedi ("python.jediEnabled": true; the default), pylint reported the error. Using the language server ("python.jediEnabled": false), it reported the error. Either way I got the same error you reported.

You are correct that the sys.path manipulation in mypackageA/__init__.py should be reflected in all imports of mypackageA submodules. I consider this a bug. However, there is a real possibility that it is a bug in pylint/language server and will have to be resolved upstream. There may be workarounds we can do in the extension. We'll take a look.


# main.py

from mypackageA import app
app.App()

# mypackageA/__init__.py

import os.path
import sys
PKG_ROOT = os.path.dirname(__file__)
PROJECT_ROOT = os.path.dirname(PKG_ROOT)
LIB = os.path.join(PROJECT_ROOT, 'external repo', 'lib')
sys.path.insert(0, LIB)

# mypackageA/app.py

from config import Config  # <--- error marked here
class App:
    def __init__(self):
        cfg = Config()
    ...

# external repo/lib/config.py

class Config:
    ...

@ericsnowcurrently ericsnowcurrently added area-linting needs PR bug Issue identified by VS Code Team member as probable bug and removed triage feature-request Request for new features or functionality labels Jun 27, 2019
@ericsnowcurrently ericsnowcurrently removed their assignment Jun 27, 2019
@farleylai
Copy link
Author

Thanks for your timely confirmation.
I really hope after the issue is fixed, Go to definition would also work as expected.

@rrmistry
Copy link

rrmistry commented Jul 3, 2019

Hello @ericsnowcurrently , I have a similar issue that may add value to this conversation.

I have a simple project setup

Project root
+-- mymodule
     +-- config.py
     +-- mymodule.py

I am trying to import config.py in mymodule.py using import config since it is in the same folder. This seems to work if I run from python directly, but fails when running through the IPython Kernel:
VSCode_RelativeImport_Error

Repo Location: https://github.com/rrmistry/vscode-python-relative-import


[EDIT]: I discovered the root cause. Please see comment #6241 (comment)

@rrmistry
Copy link

rrmistry commented Jul 3, 2019

Hello @ericsnowcurrently , please disregard my previous comment.

The issue I faced was trivial. The IPython Kernel's current working directory was on the Project root instead of my module sub-folder.

I discovered the correct approach upon importing a working jupyter notebook using the extension.

For reference, what is required is to add the following to the begining of mymodule.py:

#%% Change working directory from the workspace root to the .py file location.
import os
try:
	os.chdir(os.path.join(os.getcwd(), 'mymodule')) # change directory
	print(os.getcwd())
except:
	pass # ignore error in-case re-running this cell
	# (e.g. new cwd = `Project root/mymodule/mymodule`)

If no objections, I'll leave these comments here for anyone else who may run into the same issue. Thanks,

@MikhailArkhipov
Copy link

It is unlikely that LS will be actually executing Python code to get sys.path (running random code is a security issue). However, there is improved handling of imports as well as troubleshooting guide here https://github.com/microsoft/python-language-server/blob/master/TROUBLESHOOTING.md.

@nunesvictor
Copy link

In my case the problem was solved replacing

"python.autoComplete.extraPaths": [
    "${workspaceFolder}/src"
],

for

"python.autoComplete.extraPaths": [
    "./src"
],

in my .vscode/settings.json file.

That's a little odd but it worked

@farleylai
Copy link
Author

@MikhailArkhipov
Security should not be a concern if VSCode or LS makes its best effort to perform compiler analysis for potential changes to sys.paths in __init__.py instead of running __init__.py directly.

@nunesvictor
This feels more like an workaround because the package developer already sets the necessary sys.paths in __init__.py. The intention is NOT to have users to change their local settings.

@aripalo
Copy link

aripalo commented Oct 21, 2019

I have a bit similar situation, my use case is that I am developing a polyglot monorepo containing multiple AWS Lambda functions. Depending on the use case, they are written in Go, TypeScript and Python.

I try to keep all the functions "contained" in such a way that the folder containing the AWS Lambda function source code has the manifest-file (requirements.txt in Python's case) and all its dependencies within it. This works well for AWS Lambda as it requires having all the dependencies bundled in the deployment package and also for local development with lambci/lambda Docker.

I've initialized each of the Python AWS Lambda functions with their own virtual environment python3 -m venv .venv and then installed vendor dependencies with pip install --upgrade -r requirements.txt (these steps I actually execute inside lambci/lambda Docker containers).

Then in the AWS Lambda source code stored in src/lambda/<function-name>/main.py I have the following:

import logging as log
log.getLogger().setLevel(log.DEBUG)

from pathlib import Path
path = Path(__file__).parent.absolute()
sys.path.insert(0, str(path) + "/.venv/lib/python3.7/site-packages")
log.debug("sys.path:" + str(sys.path))

# After this I import some local dependencies from .venv
import boto3 # Local boto3 version imported as I require newer version than the one in Lambda runtime environment
import cfnresponse
# ... and possibly many more depending on the use case.

Where of course the <function-name> is replaced with the actual function name.

This works fine for the local development in lambci/lambda Docker and in actual AWS Lambda environment.

The problem is that VS Code linter and intellisense doesn't keep up with that solution.

I can get the VS Code linter & intellisense working by defining in ${workspaceFolder}/.env file the following:

PYTHONPATH=src/lambda/<function-name>/.venv/lib/python3.7/site-packages

The problem here is that I may have several of these functions, let's say foo and bar. Of course one can set multiple folders into PYTHONPATH separated by os.pathsep, which would mean I'd have to configure:

PYTHONPATH=src/lambda/foo/.venv/lib/python3.7/site-packages:src/lambda/bar/.venv/lib/python3.7/site-packages

… to get the dependencies of foo and bar AWS Lambda functions to work in VS Code.

Unfortunately this becomes problematic as I might have quite many different AWS Lambda functions, so I'd have to define all the different AWS Lambda source code folders into PYTHONPATH which will become tedious task.

So it'd be nice if there was a singular method for defining the dependencies path for these Python AWS Lambda functions, that would work for the actual code (in AWS Lambda & in lambci/lambda) and in VS Code.

@brettcannon
Copy link
Member

Parsing source to extract sys.path manipulations is unfortunately something we will not be implementing.

@ghost ghost removed the needs PR label Mar 11, 2020
@lock lock bot locked as resolved and limited conversation to collaborators Mar 21, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
area-linting bug Issue identified by VS Code Team member as probable bug
Projects
None yet
Development

No branches or pull requests

9 participants