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

Detect package when analysing import statements #36

Open
lfurrer opened this issue Jun 2, 2017 · 24 comments · May be fixed by #52
Open

Detect package when analysing import statements #36

lfurrer opened this issue Jun 2, 2017 · 24 comments · May be fixed by #52

Comments

@lfurrer
Copy link

lfurrer commented Jun 2, 2017

In parallel to an issue over at AtomLinter, SublimeLinter-pylint produces spurious error messages when analysing dotted relative import statements inside a Python package.

For example, consider the following package (directory) structure:

working directory
| - package
|   | - foo
|   |   | - bar.py
|   |   | - baz.py

In the module bar.py, it's totally fine (AFAIK) to write:

from . import baz

However, if you simply run

pylint package/foo/bar.py

it will show an error "Attempted relative import beyond top-level package", because it doesn't recognize bar.py is part of a package.
This is also what I see in the editor when SublimeLinter-pylint is enabled.

In order for pylint to acknowledge the package structure, you have to run

pylint package.foo.bar

Is there any way for SublimeLinter to detect packages and adjust the calls to pylint accordingly?
Or is there at least a way to tell SublimeLinter-pylint where the package root is?

Please note: This is all Python 3.

@NotSqrt
Copy link
Member

NotSqrt commented Jun 6, 2017

Hi,

There's a very useful setting paths that may help.

One of the reasons behind relative import errors is that the "background" lint mode of sublime linter uses a tmp copy of the file when linting your changes before saving the file. If you save the changes, the real file is used, resulting in different errors.

@lfurrer
Copy link
Author

lfurrer commented Jun 6, 2017

Hi,

thanks for the hint. This is {"SublimeLinter": {"linters": {"pylint": {"paths": []}}}}, right?
Is it used like the PYTHONPATH env var, ie. should it contain the parent directory of a package used?

I'm aware of the tmp copy effect: the error changes to "Cannot import" while there are unsaved changes.
The issue described here applies to "saved state".

I narrowed the problem further down: SublimeLinter-pylint correctly recognises "regular packages", even without explicitly listing them in paths. The problem only occurs with the so-called "namespace packages", a relatively recent feature (well, Python 3.3...). In namespace packages, there are no __init__.py modules, so basically any directory tree with .py files in it can be a package now. So, correctly recognising them might become a bit brittle...

@braver
Copy link
Member

braver commented Jan 9, 2018

I think this is mostly a duplicate of #16.

@braver braver closed this as completed Jan 9, 2018
@lfurrer
Copy link
Author

lfurrer commented Jan 9, 2018

As far as I understand #16, it turned out that the error only appeared when the file was unsaved. The error here only appears when the file is saved. And the error in #16 was "No name X in module Y", while here it is "Attempted relative import beyond top-level package".

@braver braver reopened this Jan 9, 2018
@braver
Copy link
Member

braver commented May 17, 2018

Part of me thinks we should just make this plugin file-only. It's much more powerful when running on the actual file, it's just not a good match for the tempfile approach. @kaste what about you?

@kaste
Copy link
Contributor

kaste commented May 17, 2018

I'm not a fan of pylint. It's super slow and not even correct.

@braver
Copy link
Member

braver commented May 17, 2018

I don’t disagree. I’ll make it file-only so it’s consistent.

@lfurrer
Copy link
Author

lfurrer commented May 24, 2018

I hate to insist, but the behaviour described here ("Attempted relative import beyond top-level package" for namespace packages, ie. those without an __init__.py file) appears when the file is saved, ie. when pylint is run on the actual file, not the tempfile. I'm pretty sure #51 has no effect on the behaviour described here.

@braver braver reopened this May 24, 2018
@braver
Copy link
Member

braver commented May 24, 2018

Oh, good point. Well, someone who actually uses this and knows more about Pylint might be able to drum up a solution?

@kaste
Copy link
Contributor

kaste commented May 25, 2018

Hm, ’foo/init.py’ is missing in your first post.

(Backticks don't work on mobile.)

@lfurrer
Copy link
Author

lfurrer commented May 25, 2018

@kaste that's exactly the point, a namespace package is a package without __init__.py.

@kaste
Copy link
Contributor

kaste commented May 25, 2018

I assume package to be the namespace package then package goes without the __init__.py. But foo has an __init__.py.

@NotSqrt
Copy link
Member

NotSqrt commented May 25, 2018

I think __init__.py are no longer mandatory at any module level with python3.

@kaste
Copy link
Contributor

kaste commented May 25, 2018

Don't think so and never heard of it. Any references?

@kaste
Copy link
Contributor

kaste commented May 25, 2018

You're reading it wrong. OP wants to do relative imports like from . import baz. So foo must be a package bc a package groups related modules, and a package has a __init__ file to mark it as such.

The feat OP mentions is 'namespace package' which is a totally niche feat. The stackoverflow answers are misguiding.

@kaste
Copy link
Contributor

kaste commented May 26, 2018

FWIW, I think in SL4 you can also set PYTHONPATH directly 🤔

linters:
	pylint:
		env: {
			"PYTHONPATH": "/foobar/:$PYTHONPATH"
		}

@lfurrer
Copy link
Author

lfurrer commented May 31, 2018

@kaste thanks, I added the following to my project settings, did you mean this?

{
    "SublimeLinter": {
        "linters": {
            "pylint": {
                "env": {
                    "PYTHONPATH": "/absolute/path/to/project-root/:$PYTHONPATH"
                }
            }
        }
    }
}

Because it doesn't have the desired effect – the pylint message stays the same (after restarting the editor). Maybe PYTHONPATH isn't the right env var for pylint for determining the package root?

@kaste
Copy link
Contributor

kaste commented Jun 8, 2018

@En-dash Yes, that's what I had in mind. Using PYTHONPATH should behave similar to our paths setting.

But I also stated that namespace packages contain 'normal' packages, basically __init__ is not optional.

@lfurrer
Copy link
Author

lfurrer commented Jun 8, 2018

Granted: reading the PEP, it doesn't sound like the (implicit) namespace package was created in order to make __init__.py optional. But as far as I understand, that was its (intended or unintended) side effect.

I'm not saying that this is a major issue. But the example from the OP is accepted by the pylint executable, and it runs by Python as well. I just ran these bash commands:

mkdir -p package/foo
echo 'y = 3' > package/foo/baz.py
echo 'from . import baz' > package/foo/bar.py

and I ran these three tests (Python version is 3.5.2):

pylint package/foo/bar.py
pylint package.foo.bar
python3 -c 'import package.foo.bar as bar; print(bar.baz.y)'

In the first case, pylint complained about the attempted relative import, but it didn't in the second case. The third test prints out "3", as expected.

The SublimeLinter-pylint plugin always behaves like the first test, irrespective of the settings PYTHONPATH.

@kaste
Copy link
Contributor

kaste commented Jun 8, 2018

Okay, I'm relatively convinced now that I'm wrong and you and @NotSqrt are correct.

However, I think it's pylint's fault. The 'normal' form for pylint is that you give it a package/module string as argument like (foo.bar.baz). If you instead give it a path it converts this path to a module string for you. But as far as I can see, this is not broken but just not implemented. They ask pkg_resources for declared namespaces if a dir does not contain __init__, so it seems that implicit namespaces are not supported.

@kaste kaste linked a pull request Jun 8, 2018 that will close this issue
@kaste
Copy link
Contributor

kaste commented Jun 8, 2018

PR #52 for a first? implementation. Take a look!

@alexchandel
Copy link

pylint-dev/pylint#2967 has been opened. No updates.

@kaste
Copy link
Contributor

kaste commented Jul 29, 2020

I still like my #52 btw.

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

Successfully merging a pull request may close this issue.

5 participants