Skip to content

Conversation

@bnavigator
Copy link

Further work on palantir#775 which helped me for the OpenSUSE package (where Jedi has a version 55 commits after 0.16.0)

* Fix hover request for numpy alias (np) and ufuncs

* Change hover test string
@bnavigator bnavigator changed the title Jedi Remove deprecated calls for Jedi >= 0.16.0 Apr 9, 2020
Copy link

@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.

Thanks @bnavigator for your help! I left some comments for you.

positional_args = [param for param in d.params
sig = d.get_signatures()
if (include_params and sig and not is_exception_class(d.name)):
positional_args = [param for param in sig[0].params
Copy link

@ccordoba12 ccordoba12 Apr 13, 2020

Choose a reason for hiding this comment

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

Why are you taking here only the first signature? Also, could you provide an example of an object with multiple signatures?

Copy link
Author

Choose a reason for hiding this comment

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

Because PyLS expects only one for completion, but the newer Jedi provides a list of signatures.
https://jedi.readthedocs.io/en/latest/docs/api.html#jedi.Script.get_signatures

I am pretty sure there are cases with multiple signatures for a completion.
https://jedi.readthedocs.io/en/latest/docs/api-classes.html#jedi.api.classes.BaseName.get_signatures

    def get_signatures(self):
        """
        Returns all potential signatures for a function or a class. Multiple
        signatures are typical if you use Python stubs with ``@overload``.

        :rtype: list of :class:`BaseSignature`
        """
        return [
            BaseSignature(self._inference_state, s)
            for s in self._get_signatures()
        ]

params = ', '.join([param.name for param in definition.params])
sig = definition.get_signatures()
if definition.type in ('function', 'method') and sig:
params = ', '.join([param.name for param in sig[0].params])

Choose a reason for hiding this comment

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

Also here, why sig[0]?

Copy link
Author

Choose a reason for hiding this comment

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

see above

Comment on lines +216 to +218
script = self.jedi_script()
return script.get_names(all_scopes=all_scopes, definitions=definitions,
references=references)

Choose a reason for hiding this comment

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

Why did you ditch environment here?

Copy link
Author

@bnavigator bnavigator Apr 13, 2020

Choose a reason for hiding this comment

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

Because environment is already pulled from the configuration in self.jedi_script() and is passed to the jedi.Script() constructor there. The .get_names() method does not have an environment parameter.

@bnavigator
Copy link
Author

Jedi 0.17.0 came out and bbfcce1 is an intermediate commit. This is still WIP, as stated in palantir#744

@bnavigator
Copy link
Author

Rebased and completed. See palantir#744 (comment)

Foxboron and others added 7 commits April 16, 2020 12:39
Signed-off-by: Morten Linderud <[email protected]>
fix path and project parameters for jedi_script
remove deprecated usages() calls
source is deprecated in 0.17.0 in favor of code
but code did not exist in 0.16.0
sometimes jedi reports some keywords first
@bnavigator
Copy link
Author

I reused the same branch for palantir#781 and after the rebase this does not merge into 755 anymore. closing.

@bnavigator bnavigator closed this Apr 16, 2020
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.

4 participants