microsoft / vscode-dev-containers

NOTE: Most of the contents of this repository have been migrated to the new devcontainers GitHub org (https://github.com/devcontainers). See https://github.com/devcontainers/template-starter and https://github.com/devcontainers/feature-starter for information on creating your own!
https://aka.ms/vscode-remote
MIT License
4.71k stars 1.41k forks source link

Python- pylint with extra modules #267

Closed bhack closed 4 years ago

bhack commented 4 years ago

Steps to Reproduce:

  1. Add a dependency in requirements.txt and uncomment https://github.com/microsoft/vscode-dev-containers/blob/master/containers/python-/.devcontainer/devcontainer.json#L38
  2. Pylint is not able to resolve that dependency but code run correctly (so the module is available).
Chuxel commented 4 years ago

This is expected. pylint is installed via pipx in the image to avoid global module conflicts. These are in venvs in /usr/local/py-utils.

See the discussion here.

This allows all tools to be present without impacting the global environment and enables user installed versions to be specified which override those in the utilities folder.

Chuxel commented 4 years ago

Is there a specific issue this is causing or just an observation?

bhack commented 4 years ago

Yes basically pylint is not working with required modules. What is the best practices to add extra modules and use pylint? Cause of course requirements.txt in the template could seems to users the most natural way to add extra dependencies.

Chuxel commented 4 years ago

Can you post the specific thing you are trying to install?

The short answer is that you can update the pipx environment with pipx inject pylint <other things to install>, but I can provide a more complete example if you can give me a few more details.

bhack commented 4 years ago

What I meant is that we could probably change this line with pipx? Cause when Vsocde is going to automatically add the devcontainer.json to your repo then it is quite natural for user to add dependencies in requirements.txt. But if the user uncomment the default line in the automatically added devcontainer.json pylint is not working cause is pip 3.

Chuxel commented 4 years ago

Hmmm. The idea behind pipx is its an isolated environment separate from your application dependencies, I wouldn't expect we'd want to do that. If pylint can't be isolated, then we'd want to pip install it.

@brettcannon Given the original suggestion, what are your thoughts on this feedback?

bhack commented 4 years ago

@Chuxel I understand but we need to find a general solution that could isolate tools but also let the IDE to work with user modules quite easily with python tools.

Chuxel commented 4 years ago

Yes, @brettcannon is from the Python team.

I'm still not quite sure what problem you're encountering. If I go and grab https://github.com/microsoft/vscode-remote-try-python and open the container, I end up with a project with the flask module installed globally using this image. In this case, pylint is able to find it without issue.

Flask is installed via pip3 install -r requirements.txt.

Or do you mean something different by "required modules"?

If you could include the specific code and requirements.txt file that you are seeing the problem with, we could zero in on a solution. Originally it sounded like you were expecting installations from pip3 install -r requirements.txt to pick up modules already installed by pylint. That won't happen, but pylint should be seeing your globally installed packages.

bhack commented 4 years ago

There is something different there from https://github.com/microsoft/vscode-dev-containers/blob/master/containers/python-/. Cause adding the same modules in the flask example is working there.

Chuxel commented 4 years ago

@bhack Thanks for confirming. I will dig in to see if I can determine the difference with a fresh project.

Chuxel commented 4 years ago

@bhack Okay, I was able to repo. Try adding this to your Dockerfile to see if it fixes it:

USER root
RUN pipx uninstall pylint  && pipx install --system-site-packages pylint
bhack commented 4 years ago

@Chuxel yes it is ok with this.

Chuxel commented 4 years ago

Great! Will get a patch queued up.

//cc: @chrmarti as FYI

Chuxel commented 4 years ago

I just merged in a fix - once CI runs you can try using the dev version of the image.

e.g.

mcr.microsoft.com/vscode/devcontainers/python:dev-3

We'll get this in the next update to Remote - Containers as well.

Thanks for reporting this - it was clearly hidden behind how the try repo project was setup.