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

relative imports/includes fail when linting #1175

Closed
belonesox opened this Issue Mar 20, 2016 · 15 comments

Comments

Projects
None yet
4 participants
@belonesox

belonesox commented Mar 20, 2016

Short Summary

Let is consider sample FooBar module in Python
https://github.com/belonesox/foobarpy/blob/master/mods/foom/foom.py

with some relative imports.
If we call
pylint foom.py
— no problem with relative imports.
If we use Komodo IDE/Komodo Edit
we have Pylint errors like
Ln: 7, pylint: E0401 Unable to import '#iw6prl.foo2' (ERROR)

Steps to Reproduce

Expected results

No errors in import,
try just run
pylint foom.py
— should be no import errors.

Actual results

d

Ln: 7, pylint: E0401 Unable to import '#iw6prl.foo2' (ERROR)

Platform Information

Komodo Edit or IDE?
Komodo IDE 9.3.2
Komodo Edit 9.3.2
Komodo Version? 9.3.2
Operating System (and version)?
Linux, Windows 7

Additional Information

Looks like the problem Komodo run pylint on some temporary file somewhere,
and that this problem is rather old (https://community.activestate.com/node/9417 + bugs in closed bugzilla products).

I hope the problem should be finally fixed (I am tired to set "pylint: disable" for every relative import).

-- eg. Error logs, screenshots, workarounds --

Dev Edit

This will affect any linter that supports relative imports.

@belonesox

This comment has been minimized.

belonesox commented Mar 20, 2016

Tracing pylint calls I see, that Komodo use temporary filename in some temp dir for saving unsaved text and pylint checking.
Of course, this is wrong way, any relative imports will be "false negative".

@Naatan

This comment has been minimized.

Member

Naatan commented Mar 21, 2016

Seems we may finally have a test case for #14

@belonesox could you comment on Todd's statement?

As far as I understand, in Python it is illegal to use a relative import to refer to modules located in a different directory on the filesystem. (Moreover, import .some is also an illegal statement.) Therefore, your attempt to import modules.first.some via from .some import ... in a file not located in modules/first is invalid Python.

@belonesox

This comment has been minimized.

belonesox commented Mar 21, 2016

@belonesox could you comment on Todd's statement?

It is not true.
https://www.python.org/dev/peps/pep-0328/

«from ...xxx import yyy» syntax works for Python 2 (since 2.5) and Python 3, and even supported by pylint checker.

@Naatan Naatan added Type: Bug and removed Pending: Response labels Mar 21, 2016

@Naatan Naatan added this to the 10.0 milestone Mar 21, 2016

@Naatan

This comment has been minimized.

Member

Naatan commented Mar 21, 2016

@mitchell-as can you look into integrating #14 ?

@mitchell-as

This comment has been minimized.

Member

mitchell-as commented Mar 21, 2016

@Naatan yes I can, but that will not fix this ticket, which has nothing to do with #14. (Linting vs. codeintel).

@mitchell-as

This comment has been minimized.

Member

mitchell-as commented Mar 21, 2016

Reproduced in Python3 with pylint 1.5.4.

@belonesox

This comment has been minimized.

belonesox commented Mar 22, 2016

Thank you, but I should note that this is only workaround, such way can not handle relative wildcard imports, and "Komodo pylint" will raise errors like that:

d

( https://github.com/belonesox/foobarpy/blob/master/mods/foom/foom2.py, syntaxically correct, OK for pylint running on same directory )

@mitchell-as mitchell-as reopened this Mar 22, 2016

@mitchell-as

This comment has been minimized.

Member

mitchell-as commented Mar 22, 2016

Thanks for the additional case. It looks like we'll have to come up with something more robust.

@mitchell-as mitchell-as self-assigned this Mar 22, 2016

@Naatan

This comment has been minimized.

Member

Naatan commented Mar 22, 2016

This could be useful:

    --init-hook=<code>  Python code to execute, usually for sys.path
                        manipulation such as pygtk.require().

To alter the cwd.

@mitchell-as

This comment has been minimized.

Member

mitchell-as commented Jun 20, 2016

Nope, using --init-hook to set the cwd is no good. I'm pretty sure that in this case (with relative imports) the linter needs to be run on the file directly as opposed to on a temporary file. I think Pylint is getting confused with module names:

[2016-06-20 12:58:32,027] [DEBUG] koPythonLinter: stdout: '************* Module #O2PYXI\nC0103: 1,0: Invalid module name "#O2PYXI"\n [...]

(That weird module name is the name of the temporary file.)

If I run Pylint directly on the file in its directory, I get the expected results. However, none of our linters run directly on files -- they always create temporary files and lint those. I'm not sure of the ramifications if we linted files (or at least Python files) directly. @Naatan what do you think?

@mitchell-as

This comment has been minimized.

Member

mitchell-as commented Jun 20, 2016

I might be able to lint directly on files conditionally: if I scan the text to lint and find a from . line I can lint on the file directly, but otherwise fall back on the default lint a temporary file behavior.

@Naatan Naatan modified the milestones: 10.1, 11 Aug 15, 2016

@Naatan Naatan changed the title from Pylint + KomodoEdit/Komodo IDE + relative imports = fail to relative imports/includes fail when linting Aug 22, 2016

@mitchell-as

This comment has been minimized.

Member

mitchell-as commented Sep 21, 2016

@Naatan ping. I see you upped the severity, but didn't respond to my question.

@Defman21

This comment has been minimized.

Contributor

Defman21 commented Sep 21, 2016

I think that require_relative in Ruby also is being covered by this issue.

@Naatan

This comment has been minimized.

Member

Naatan commented Sep 21, 2016

Ramifications I can think of:

  • linters could write temp files relative to the file they are linting, this would be bad
  • dirty files should not be saved to perform linting (at least not in the same file path)

I would suggest adding a pref that lets people say "save temp linting files in the same directory", but given the miniscule amount of people that actually customize these prefs I'm not convinced this is worth the effort.

I'd imagine we could address this more intelligently if and when we write our own linters?

@mitchell-as

This comment has been minimized.

Member

mitchell-as commented Sep 21, 2016

While we'd have more control over this if we wrote our own linters, it would likely be a big undertaking.

I think at this point we can conclude there is no way of catching every case of relative imports. So far it looks like we only cannot handle wildcard relative imports. I'd recommend just putting # pylint: disable=WXXXX at the top of the source, where XXXX is the number of the specific warning from pylint.

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