Return types from numpydocs and compiled objects return types from docstrings #868

bcolsen · 2017-02-14T05:15:57Z

In this PR I have updated the docstrings.py from @Erotemic in PR: #796 so that it merges against the changes that have been made to the current dev branch since the original commits. I have also removed the google docstrings from this PR as I don't have the experience necessary to test these docstrings. However, It should be quite simple for @Erotemic to add this functionality as an addition PR. With this split there are less code changes, hopefully making this PR easier to merge.

This pull request solves issue #372 for numpy complied objects.

Numpy functions such as numpy.array return an numpy.ndarray object resulting in completions like this:

import jedi; jedi.Script('import numpy as np; np.array([1,2,3]).a').completions()
[<Completion: all>,
<Completion: any>,
<Completion: argmax>,
<Completion: argmin>,
<Completion: argpartition>,
<Completion: argsort>,
<Completion: astype>]

I have not yet written a test since I need to import numpydocs and numpy to do such.
A standard python library with complied objects and sphinx documentation would be a better test.

bcolsen · 2017-02-15T20:53:51Z

I forgot to add the testing file to this PR.
I'll add the tests with the google docstrings test removed.

Is there a way to make a complied object to test the compiled object type return?

ccordoba12 · 2017-04-08T20:41:55Z

@davidhalter, could you take a look at this one please?

We're maintaining a big patch in Spyder to make this work, and we would really like to drop it in favor of using an upstream release.

davidhalter · 2017-04-19T22:53:01Z

Seriously I'm sorry. I just don't have the time to review this. There were some other priorities (including real life) that made it hard to review this. It's really the next thing on my list once I have time.

I'll try ASAP.

ccordoba12 · 2017-04-19T22:54:55Z

Thanks for the answer, and good to know this one is high in your TODO list.

davidhalter · 2017-04-30T18:53:30Z

Sorry again for taking so long. I finally had time to review #796 and have tried to reach the author. If we don't get that working in the next few weeks, we'll just merge this one. I hope this is OK for you.

ccordoba12 · 2017-05-01T18:13:37Z

Thanks David, that's fine for us.

ccordoba12 · 2017-05-02T21:51:08Z

@bcolsen, could you update this PR in the meantime? It has some conflicts with master.

bcolsen · 2017-05-02T22:52:24Z

@ccordoba12, yeah I'll try a rebase on the dev branch.

bcolsen · 2017-05-03T05:46:29Z

Well that was more difficult than I thought it would be.

There were some breaking changes in docstrings.py, but I fixed it.

ccordoba12 · 2017-05-07T17:18:08Z

@davidhalter, now that the original author of #796 has confirmed that he doesn't have time to update his work, I think we can proceed with this one.

davidhalter

Ok. I have finally reviewed this one. I stopped at one point, I will probably continue in a second iteration.

My general view is that the quality of this patch is not good enough. There are certain hacks and they are not even documented. I don't have a problem if you don't know how to work with the Jedi internals, but there's also a lot of small things that got changed along the lines. There's no need for changing anything except the numpy stuff. I'm not even against some of those changes in general, but please do it in a separate pull request, it was reaaaaally annoying to review this one and it's also why it took me so long to finally look at it. Small pull requests usually take me 2 days...

There's 3 more things that I haven't mentioned in the comments:

Document how multiple returns in the documentation behaves and how a tuple behaves. It's very unclear from https://github.com/numpy/numpy/blob/master/doc/HOWTO_DOCUMENT.rst.txt how this whole thing works.
Please include numpydocs in tox.ini as a dependency so it automatically gets tested. Also add a comment there.
I want more tests. Simple tests like described in the comments, but I want those cases with multiple returns, yields and whatever tested.

A lot of things here need changing before I merge this. I would even think that it's probably easier to restart from zero and just copy the few good parts.

davidhalter · 2017-07-29T08:34:24Z

test/test_evaluate/test_docstring.py

@@ -150,3 +152,68 @@ def foobar(x, y):
        assert 'capitalize' in names
        assert 'numerator' in names
        assert 'append' in names


Please try to use pytest tests for these tests. Just write them as functions and use pytest.skipif. I'm trying to get rid of those classes, because they don't make sense with pytest.

davidhalter · 2017-07-29T08:34:58Z

test/test_evaluate/test_docstring.py

+        evaluator = script._evaluator
+        types = docstrings.find_return_types(evaluator, func)
+        assert len(types) == 1
+        assert types[0].base.obj is builtins.int


Same as above: Also use the Jedi API directly here.

davidhalter · 2017-07-29T08:35:53Z

test/test_evaluate/test_docstring.py

+        y_base_objs = set([t.base.obj for t in y_type_list])
+        x_base_objs = set([t.base.obj for t in x_type_list])
+        assert x_base_objs == set([builtins.int, builtins.str, builtins.list])
+        assert y_base_objs == set([builtins.int, builtins.str])


This is way too much code. It will just make refactoring harder. Please test the jedi api directly by using jedi.Script (as in the test above).

davidhalter · 2017-07-29T08:39:55Z

jedi/evaluate/compiled/__init__.py

        if self.type != 'funcdef':
            return
-
+        types = set([])


Please undo this. Defining a set is not necessary. In Python 3 we would just yield from and in 2 we have to work around with

for type_ in docstrings.infer_return_types(self): yield type

davidhalter · 2017-07-29T09:05:51Z

jedi/evaluate/docstrings.py

+        Search `docstr` (in numpydoc format) for type(-s) of `param_str`.
+        """
+        doc = NumpyDocString(docstr)
+        returns = doc._parsed_data['Returns']


I just realized again that we're using a non-public API here. Can we please have a try/except around it and catch both key error and

try: # This is a non-public API. If it ever changes we should be prepared and return gracefully. returns = doc._parsed_data['Returns'] returns += doc._parsed_data['Yields'] except (KeyError, AttributeError): return []

Please also change this for the param version of this function.

davidhalter · 2017-07-29T09:36:25Z

jedi/evaluate/docstrings.py

+                p_type = p_name
+                p_name = ''
+
+            m = re.match('([^,]+(,[^,]+)*?)$', p_type)


WTF does this do?

Nothing I think. I removed it and it still works.

It seems to be copied from the exiting numpydocs params function:
m = re.match('([^,]+(,[^,]+)*?)(,[ ]*optional)?$', p_type)
which seems like a complicated way of removing the , optional from the parameter type string.

davidhalter · 2017-07-29T09:38:35Z

jedi/evaluate/docstrings.py

+            # python2 does not support literal set evals
+            # workaround this by using lists instead
+            p_type = p_type.replace('{', '[').replace('}', ']')
+        types = set(type(x).__name__ for x in literal_eval(p_type))


Jedi has it's own ways of parsing stuff. No need for this. I'm not sure what this does anyway, but I'm pretty sure that it's not the way how you should do things.

Numpydoc allows types to be a set of literals like:

order : {'C', 'F', 'A'}

It works like this:

>>> from ast import literal_eval >>> set(type(x).__name__ for x in literal_eval("['x', [], 2.3]")) {'float', 'list', 'str'}

This code in already in docstrings.py:
https://github.com/davidhalter/jedi/blob/dev/jedi/evaluate/docstrings.py#L60

All this being said I'm sure that there is already a way to do this in jedi

bcolsen · 2017-08-08T06:12:22Z

@davidhalter Thank you for looking into this.

I have read your comments and did more research into what this code is doing since I have been mostly porting it over up until now.

I have already started to simplify the code and write my own tests.

davidhalter · 2017-08-08T17:00:28Z

Let me know when you're ready!

bcolsen · 2017-08-09T06:35:40Z

It's ready. I reduced and rewrote the code using yields to return multiple types instead of lists.

The _expand_typestr function is likely the worst but it is tested and works to parse the numpy type strings for multiple type.

I made a bit of a mess in tox.ini it seems that numpydocs doesn't support py2.6 or py3.3 although the next version of jedi might drop those versions as well.

I also can't seem to make a test for yields. Jedi returns a generator object.

Using it without control over the input leads to various possible exceptions. Refs #868.

davidhalter · 2017-09-09T17:30:31Z

@bcolson Good work! Thanks. I think it's pretty cool that we have this now within Jedi.

I made one minor change: literal_eval should not be used. It leads to potential exceptions. I instead used jedi's parser (parso) to parse it and work with it. I hope that's fine. This also gets rid of the set literals hack (even though I think that was not needed in the first place, because they are not supported in 2.6 but are in 2.7. However since numpydocs doesn't work in 2.6 you can just ignore 2.6.).

I also can't seem to make a test for yields. Jedi returns a generator object.

Hmm if you're still interested, that might be true, but you can just use next or a for loop in test code to see what's in there.

bcolsen · 2017-09-10T05:10:12Z

Thanks for the review! It is cool to be completing Numpy now.

The parser code for the literal is much better than trusting eval.

Hmm if you're still interested, that might be true, but you can just use next or a for loop in test code to see what's in there.

Of course, I wasn't thinking right about that one. The tests work either way but I'll give it a shot with next.

This was referenced Feb 14, 2017

PR: Return types for CompiledObjects using docstrings.py #818

Closed

Added google style docstr support + numpy docstr fixes #796

Closed

wizmer mentioned this pull request Apr 16, 2017

doesn't work with numpy pythonic-emacs/anaconda-mode#243

Closed

bcolsen force-pushed the numpydoc branch from c0959c2 to 7c0be78 Compare May 3, 2017 05:40

davidhalter requested changes Jul 29, 2017

View reviewed changes

Numpydocs and compiled objects return types

d19a97f

bcolsen force-pushed the numpydoc branch from 8135c40 to d19a97f Compare August 9, 2017 04:49

bcolsen added 3 commits August 8, 2017 23:13

Numpydocs doesn't support 2.6 or 3.3

4f96cdb

fix test skip and py3.6

77d6de0

add numpydoc to cov in tox.ini

38a690b

Added Yields test

3422b21

davidhalter added a commit that referenced this pull request Sep 9, 2017

Don't use literal_eval

d47804e

Using it without control over the input leads to various possible exceptions. Refs #868.

davidhalter merged commit 3422b21 into davidhalter:dev Sep 9, 2017

Erotemic mentioned this pull request Sep 10, 2017

Added support for google-style docstrings #963

Closed

bcolsen deleted the numpydoc branch January 26, 2019 17:05

gepcel mentioned this pull request Apr 1, 2019

Ask the IPython kernel for editor code completions spyder-ide/spyder#5252

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Return types from numpydocs and compiled objects return types from docstrings #868

Return types from numpydocs and compiled objects return types from docstrings #868

bcolsen commented Feb 14, 2017 •

edited

Loading

bcolsen commented Feb 15, 2017

ccordoba12 commented Apr 8, 2017

davidhalter commented Apr 19, 2017

ccordoba12 commented Apr 19, 2017

davidhalter commented Apr 30, 2017

ccordoba12 commented May 1, 2017

ccordoba12 commented May 2, 2017

bcolsen commented May 2, 2017

bcolsen commented May 3, 2017

ccordoba12 commented May 7, 2017

davidhalter left a comment •

edited

Loading

davidhalter Jul 29, 2017

davidhalter Jul 29, 2017

davidhalter Jul 29, 2017

davidhalter Jul 29, 2017

davidhalter Jul 29, 2017

davidhalter Jul 29, 2017

bcolsen Aug 8, 2017

davidhalter Jul 29, 2017

bcolsen Aug 8, 2017

bcolsen commented Aug 8, 2017

davidhalter commented Aug 8, 2017

bcolsen commented Aug 9, 2017

davidhalter commented Sep 9, 2017

bcolsen commented Sep 10, 2017

Return types from numpydocs and compiled objects return types from docstrings #868

Return types from numpydocs and compiled objects return types from docstrings #868

Conversation

bcolsen commented Feb 14, 2017 • edited Loading

bcolsen commented Feb 15, 2017

ccordoba12 commented Apr 8, 2017

davidhalter commented Apr 19, 2017

ccordoba12 commented Apr 19, 2017

davidhalter commented Apr 30, 2017

ccordoba12 commented May 1, 2017

ccordoba12 commented May 2, 2017

bcolsen commented May 2, 2017

bcolsen commented May 3, 2017

ccordoba12 commented May 7, 2017

davidhalter left a comment • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

bcolsen commented Aug 8, 2017

davidhalter commented Aug 8, 2017

bcolsen commented Aug 9, 2017

davidhalter commented Sep 9, 2017

bcolsen commented Sep 10, 2017

bcolsen commented Feb 14, 2017 •

edited

Loading

davidhalter left a comment •

edited

Loading