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

Improve how Jedi handles Numpy #281

Merged
merged 1 commit into from
Nov 1, 2022

Conversation

gav451
Copy link
Contributor

@gav451 gav451 commented Oct 22, 2022

The documentation of jedi.settings.auto_import_modules tells that jedi may handle numpy's module globals trickery better when importing numpy instead of parsing. The example script below prints the documentation strings of one of numpy's universal functions as well as the numpy version:

#!/usr/bin/env python
import jedi, numpy

# The next setting enables jedi to find the documention string of ufuncs:
jedi.settings.auto_import_modules = ["numpy"]
# Find the documentation string of numpy.cosh by completing numpy.cos:
script = jedi.Script(code := "import numpy; numpy.cos", path="<string>")
print(script.complete(1, len(code))[1].docstring())
print(f"numpy version: {numpy.version.__version__}")

The pull request adds numpy to jedi's auto_import_modules and allows the user to modify the module list. The patch fixes:

  1. The display of those documentation strings of numpy's universal functions by Eglot and python-lsp-server is good enough for me (I am using this patch for more than a year for this reason).
  2. The incompatibility of jedi/python-lsp-server with numpy-1.23.x (this is a bonus).

Fixes #243.

Copy link
Member

@ccordoba12 ccordoba12 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hey @gav451, thanks a lot for your contribution! Besides the two suggestions I left for you below, please also update our configuration file by running

python scripts/jsonschema2md.py pylsp/config/schema.json CONFIGURATION.md

pylsp/config/schema.json Outdated Show resolved Hide resolved
pylsp/workspace.py Outdated Show resolved Hide resolved
@ccordoba12 ccordoba12 changed the title PR: improves how jedi handles numpy Improve how Jedi handles Numpy Oct 31, 2022
@gav451 gav451 force-pushed the improve-jedi-handling-numpy branch from 4398552 to 08c14ea Compare October 31, 2022 17:36
@gav451
Copy link
Contributor Author

gav451 commented Oct 31, 2022

Hey @ccordoba12, I have amended my pull request taking into account your remarks.
It was not clear to me that you prefer that contributors have to update CONFIGURATION.md and you are right about removing "gi" (I had added "gi" because jedi has jedi.settings.auto_import_modules = ['gi'] by default leaving it open for discussion).

@ccordoba12
Copy link
Member

Thanks @gav451! One last thing I forgot to mention before: could you remove the following constraint to see if our Numpy tests pass now?

"numpy<1.23",

@gav451 gav451 force-pushed the improve-jedi-handling-numpy branch from 08c14ea to a7cc7cb Compare November 1, 2022 05:50
@gav451
Copy link
Contributor Author

gav451 commented Nov 1, 2022

Thanks @ccordoba12 for your guidance (next time I know I have to do all development steps). I checked that the Numpy test passed before amending and force-pushing. See the short test summary below:

=========================== short test summary info ============================
FAILED test/plugins/test_flake8_lint.py::test_flake8_multiline - AssertionError: assert ['flake8', '-...,E,F,W,T4,B9'] == ['flake8', '-...h/...
============ 1 failed, 136 passed, 9 skipped, 7 warnings in 47.91s =============

I had also to install yapf and whatthepatch manually, since I did not succeed to edit the test section in pyproject.toml to do it automatically.

@ccordoba12 ccordoba12 added this to the v1.6.0 milestone Nov 1, 2022
@ccordoba12
Copy link
Member

Thanks @ccordoba12 for your guidance

No problem, thanks a lot for a great contribution!

I had also to install yapf and whatthepatch manually, since I did not succeed to edit the test section in pyproject.toml to do it automatically.

Yeah, you need to install pylsp with

pip install python-lsp-server[all]

to get all its dependencies for testing.

Copy link
Member

@ccordoba12 ccordoba12 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good to me now, thanks @gav451!

@ccordoba12 ccordoba12 merged commit 5c37673 into python-lsp:develop Nov 1, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Failed to load hook pylsp_completions: 'NoneType' object has no attribute 'type' when working with Numpy 1.23
2 participants