Update Jedi calls for its 0.17.0+ API

[bnavigator]

As discussed in https://github.com/palantir/python-language-server/issues/744#issuecomment-614338490, fixes #744, including most recent Jedi 0.17.0

~This is a combination of #775 and dalthviz#1~ Edit: removed the jedi master branch pulls in the CIs

[ccordoba12]

Thanks a lot @bnavigator for your help with this one! I'll review it tomorrow (I don't have time today, sorry).

[goanpeca]

Thanks a lot for this @bnavigator!!

[goanpeca]

IS this ready @ccordoba12 ?

[E1k3]

Is there any update on this or is there any way to help finalizing this pull request?

[ccordoba12]

IS this ready @ccordoba12 ?

I haven't had time to review it. Please help me with that and let me know if you need additional help.

[E1k3]

I have tested all mentioned features in a relatively simple script that imports various modules from Keras, NumPy, SciPy, Argparse and others and pyls worked flawlessly.

@ccordoba12 If more comprehensive testing or more detailed commentary on the changes are needed, please tell me in detail what you need..

[E1k3]

@ccordoba12 Is there anything to be done with this request despite merging it?

The archlinux package of pyls is currently incompatible with jedi and workarounds are on hold, because this looks finished.

[bnavigator]

@E1k3, I recommend you apply the patch directly to the Archlinux package. That's what OpenSUSE is doing.

There may be some burnt ground between @ccordoba12 and me. Chances are, he will never accept this PR despite the clear demand from the community shown by the reactions in this thread and in #744.

[goanpeca]

@bnavigator we really appreciate your help with this work. We are a bit crowded right now and in the midst of making a release. Having this PR merged could delay our release or introduce some unwanted bugs with some other libraries introspection.

There may be some burnt ground between @ccordoba12 and me. Chances are, he will never accept this PR despite the clear demand from the community shown by the reactions in this thread and in #744.

We just need to review the PR (I have not found time yet!)

We also probably need to add some simple smoke tests with libraries that have broken with the past versions of Jedi (0.16).

Thanks again for the help!

[ccordoba12]

Chances are, he will never accept this PR despite the clear demand from the community shown by the reactions in this thread and in #744.

This is not true. As @goanpeca said, we are swamped at the moment and haven't had time to review this PR, sorry. We'll try to do it and release 0.32.0 with these changes this week.

[ccordoba12]

We also probably need to add some simple smoke tests with libraries that have broken with the past versions of Jedi (0.16).

I already added tests for Numpy, Pandas and Matplotlib. One of them detected an error in Jedi 0.16, which was fixed in 0.17

[bnavigator]

Thanky you @E1k3 for answering these questions!

[bnavigator]

https://github.com/krassowski/jupyterlab-lsp/issues/270#issuecomment-626134634

Another testimony of how badly this is needed and how pinning stuff to old versions causes problems.

[ccordoba12]

Another testimony of how badly this is needed and how pinning stuff to old versions causes problems.

That's Jedi's fault by not declaring its dependency on Parso correctly (and please don't try to start another discussion about this).

[ccordoba12]

That's Jedi's fault by not declaring its dependency on Parso correctly (and please don't try to start another discussion about this).

Sorry, it wasn't. It's an error in Jedi that's fixed already (still I don't want to talk more about it, we're doing our job here trying to get this merged).

[bnavigator]

Using _workspace.root_path didn't look like such a great idea.

>       project_path = self._workspace.root_path if self._workspace else os.path.dirname(self.path)
E       AttributeError: 'MockWorkspace' object has no attribute 'root_path'

[ccordoba12]

Ok, on it. I think I know how to fix this.

[ccordoba12]

I don't think your last commit is the right solution @bnavigator. I think the problem is in MockWorkspace.

Could you leave this alone for a few minutes? Thanks!

[bnavigator]

If you allow Document(..., workspace=None) you cannot just assume that ._workspace is a workspace.

[ccordoba12]

If you allow Document(..., workspace=None) you cannot just assume that ._workspace is a workspace.

I think it's better to use something that stands for a real Workspace object (I'm almost ready with my fixes).

[fsouza]

@ccordoba12 I think what @bnavigator means is that now that workspace is required in Document, it shouldn't be an optional parameter in the constructor. As long as it's optional in the constructor, the code should support the case where it's None, even though in practice this never happens? 🤔

[ccordoba12]

You're totally @bnavigator and @fsouza! I'll do that last change here.

[bnavigator]

@ccordoba12 I think what @bnavigator means is that now that workspace is required in Document, it shouldn't be an optional parameter in the constructor. As long as it's optional in the constructor, the code should support the case where it's None, even though in practice this never happens?

Thanks for breaking it down @fsouza.

At least in the tests it happens all the time. And these are only the one-liners:

[ben@voyagerS9:~/src/python-language-server]% grep 'Document(.*)' * -R | grep -v workspace                                     [0]
test/fixtures.py:    return Document(DOC_URI, DOC)
test/plugins/test_autopep8_format.py:    doc = Document(DOC_URI, DOC)
test/plugins/test_autopep8_format.py:    doc = Document(DOC_URI, DOC)
test/plugins/test_autopep8_format.py:    doc = Document(DOC_URI, GOOD_DOC)
test/plugins/test_flake8_lint.py:    doc = Document(uris.from_fs_path(name))
test/plugins/test_flake8_lint.py:    doc = Document('', DOC)
test/plugins/test_folding.py:    doc = Document(DOC_URI, DOC)
test/plugins/test_folding.py:    doc = Document(DOC_URI, SYNTAX_ERR)
test/plugins/test_mccabe_lint.py:        doc = Document(DOC_URI, DOC)
test/plugins/test_mccabe_lint.py:    doc = Document(DOC_URI, DOC_SYNTAX_ERR)
test/plugins/test_pycodestyle_lint.py:    doc = Document(DOC_URI, DOC)
test/plugins/test_pydocstyle_lint.py:    doc = Document(DOC_URI, DOC)
test/plugins/test_pydocstyle_lint.py:    doc = Document(TEST_DOC_URI, "")
test/plugins/test_pydocstyle_lint.py:    doc = Document(DOC_URI, "")
test/plugins/test_pydocstyle_lint.py:    doc = Document(DOC_URI, "bad syntax")
test/plugins/test_pyflakes_lint.py:    doc = Document(DOC_URI, DOC)
test/plugins/test_pyflakes_lint.py:    doc = Document(DOC_URI, DOC_SYNTAX_ERR)
test/plugins/test_pyflakes_lint.py:    doc = Document(DOC_URI, DOC_UNDEFINED_NAME_ERR)
test/plugins/test_pyflakes_lint.py:    doc = Document(DOC_URI, DOC_ENCODING)
test/plugins/test_pylint_lint.py:        yield Document(uris.from_fs_path(name))
test/plugins/test_pylint_lint.py:        config, Document(uris.from_fs_path(__file__)), True)
test/plugins/test_pylint_lint.py:        config, Document(uris.from_fs_path(__file__)), False)
test/plugins/test_yapf_format.py:    doc = Document(DOC_URI, DOC)
test/plugins/test_yapf_format.py:    doc = Document(DOC_URI, DOC)
test/plugins/test_yapf_format.py:    doc = Document(DOC_URI, GOOD_DOC)
test/plugins/test_yapf_format.py:    doc = Document(uris.from_fs_path(src.strpath), DOC)
test/plugins/test_completion.py:    doc = Document(DOC_URI, DOC)
test/plugins/test_rope_rename.py:    doc = Document(DOC_URI)
test/test_document.py:    document_mem = Document(DOC_URI, u'my source')
test/test_document.py:    document_disk = Document(DOC_URI)
test/test_document.py:    doc = Document('file:///uri', u'')
test/test_document.py:    doc = Document('file:///uri', u'itshelloworld')
test/test_document.py:    doc = Document('file:///uri', u''.join(old))
test/test_document.py:    doc = Document('file:///uri', u''.join(old))

[mattst88]

Thanks a bunch to everyone involved!

If possible, please tag a release with this included. It'll help downstream Linux distros quite a lot.

[polyzen]

https://github.com/palantir/python-language-server/milestone/17

[Foxboron]

Thanks for the work getting this fixed :)

[ccordoba12]

0.32.0 is out.