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

Failing to disable pycodestyle #385

Closed
s-m-e opened this issue Aug 8, 2021 · 14 comments · Fixed by #389
Closed

Failing to disable pycodestyle #385

s-m-e opened this issue Aug 8, 2021 · 14 comments · Fixed by #389

Comments

@s-m-e
Copy link

s-m-e commented Aug 8, 2021

I just started with a clean & fresh Atom configuration and installed ide-python. I am failing to disable pycodestyle. No matter what I do, I get warnings from it in both the source code and the linter panel.

Intended behavior: Enabling ide-python with flake8, PyFlakes and pycodestyle explicitly disabled - no warnings from pycodestyle.

Actual behavior: Enabling ide-python, disabling all Python language server plugins - still warnings from pycodestyle.

"Workaround": Disabling ide-python altogether.


  • atom: 1.58.0
  • ide-python: 1.9.5
  • os: Linux x86_64
@aminya
Copy link
Member

aminya commented Aug 8, 2021

I cannot reproduce this. It might be an issue with pylsp. Let's see if others can reproduce the issue

@s-m-e
Copy link
Author

s-m-e commented Aug 8, 2021

@aminya Thanks for looking into it. That's what I thought, too, so I created a completely new virtual environment, Python 3.9.6, plus the latest pylsp & dependency packages. I am launching atom from within the virtual environment in the command line as follows: atom --foreground . I (still) can not get pycodestyle to shut up ...

@ntlspck
Copy link

ntlspck commented Aug 9, 2021

I have the same issue with pycodestyle and also mccabe, both are disabled but nevertheless I got linter errors since updating to ide-python 1.9.5

Further it seems that the ide-python settings itself are not respected, because if I enable pycodestyle, the linter error (E501) should not pop up due to my settings, where I ignore E501 and set the maxLineLength to 120.
But also with E501 set to ignored setting I got the linter errors

I also made a fresh venv with python 3.9.6 but still got the linter errors.

My current settings are:
Atom 1.58.0
ide-python 1.9.5
Python 3.9.6

"ide-python":
pylsPlugins:
flake8:
enabled: true
ignore: [
"W503"
]
maxLineLength: 120
jedi_definition:
follow_builtin_imports: true
follow_imports: true
jedi_hover: {}
mccabe:
enabled: false

threshold: 20
pycodestyle:
enabled: false
ignore: [
"E231"
"W503"
"E501"
]
maxLineLength: 120

pylint:
args: [
"-d C0114 -d C0116 -d E0401 -d W0603 -d C0301"
]
enabled: true
python: "/home/$USER/.atom/venv/bin/python"

$ pip list
Package Version


appdirs 1.4.4
astroid 2.6.6
autopep8 1.5.5
black 21.7b0
click 8.0.1
flake8 3.8.4
isort 5.9.3
jedi 0.17.2
lazy-object-proxy 1.6.0
mccabe 0.6.1
mypy-extensions 0.4.3
parso 0.7.1
pathspec 0.9.0
pip 21.2.3
pluggy 0.13.1
pycodestyle 2.6.0
pydocstyle 6.1.1
pyflakes 2.2.0
pylint 2.9.6
python-jsonrpc-server 0.4.0
python-language-server 0.36.2
regex 2021.8.3
rope 0.19.0
setuptools 56.0.0
snowballstemmer 2.1.0
toml 0.10.2
tomli 1.2.1
ujson 4.0.2
wheel 0.36.2
wrapt 1.12.1
yapf 0.31.0

linter

@ntlspck
Copy link

ntlspck commented Aug 9, 2021

Reverting commit c6a873f from #373 resolved this issue.

Workaround for Linux users:
Edit /home/$USER/.atom/packages/ide-python/dist/main.js

  mapConfigurationObject(configuration) {
    // const lsp = configuration.pyls || "pylsp";
    return {
      pyls: {
        configurationSources: configuration.pylsConfigurationSources,
        rope: sanitizeConfig(configuration.rope),
        plugins: configuration.pylsPlugins
      }
    };
  }

@aminya
Copy link
Member

aminya commented Aug 10, 2021

That commit fixes the issue where the config was not working with pylsp. Which one are you using? The deprecated pyls or pylsp?

@s-m-e
Copy link
Author

s-m-e commented Aug 10, 2021

That commit fixes the issue where the config was not working with pylsp. Which one are you using? The deprecated pyls or pylsp?

This one: python-language-server, version 0.35.1. Has it been deprecated??

@aminya
Copy link
Member

aminya commented Aug 10, 2021

The original pyls is deprecated now, and now pylsp is the one you should use. Although the configs should work for both. So if one of them is not working, that is a bug.

Installing the new pylsp:

python -m pip install 'python-lsp-server[all]'

@s-m-e
Copy link
Author

s-m-e commented Aug 10, 2021

The original pyls is deprecated now, and now pylsp is the one you should use.

Just out of curiosity: Deprecated in general or deprecated within ide-python? I am failing to find any information on deprecation in their repo.

@aminya
Copy link
Member

aminya commented Aug 10, 2021

They should add a deprecation note, but they haven't yet. See this: palantir/python-language-server#935 (comment)

Anyway, if the config isn't working for you, there is a bug somewhere. We didn't drop the support for the old pyls, and it should still continue to work.

@ntlspck
Copy link

ntlspck commented Aug 10, 2021

For those who using the deprecated pyls (python-language-server), I was able to get the config working with pyls by changing the ide-python setting "Python Language Server module" from "Default: pylsp" to "pyls"

By changing this setting there is no need to revert commit c6a873f

In general I think this issue is all about missing documentation, at least I was not aware of the deprecated pyls :)

@k2d222
Copy link

k2d222 commented Aug 10, 2021

The issue is, when ide-python fails to find pylsp it defaults to pyls but the config does not follow the fallback.
I believe we should remove the fallback to let the error message Unable to start the Python language server appear, that would tell the user that pylsp is not installed (the default) or that the field Python Language Server module is not configured properly.

@aminya
Copy link
Member

aminya commented Aug 10, 2021

Instead of using configuration input to mapConfigurationObject we should use the pyls variable. We can refactor the function, and cache it in a variable that both methods can call it.

ide-python/lib/main.js

Lines 74 to 78 in c66cecc

let pyls = atom.config.get("ide-python.pyls") || "pylsp"
// check if it exists
if (whichSync(pyls, { nothrow: true }) === null) {
pyls = "pyls"
}

So something like:

	getPyLs() {
	  if (this.pyls === undefined) {
	      let pyls = atom.config.get("ide-python.pyls") || "pylsp"
	      // check if it exists
	      if (whichSync(pyls, { nothrow: true }) === null) {
	        pyls = "pyls"
	      }
		  // cache
		  this.pyls = pyls
	   }
	   return this.pyls
	}

@aminya
Copy link
Member

aminya commented Aug 10, 2021

I made a PR. Could you test it?
#389

@github-actions
Copy link

🎉 This issue has been resolved in version 1.9.6 🎉

The release is available on:

Your semantic-release bot 📦🚀

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

Successfully merging a pull request may close this issue.

4 participants