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

Some docstrings are getting truncated in the object inspector #997

Closed
spyder-bot opened this issue Feb 17, 2015 · 16 comments
Closed

Some docstrings are getting truncated in the object inspector #997

spyder-bot opened this issue Feb 17, 2015 · 16 comments

Comments

@spyder-bot
Copy link
Collaborator

From jed.lud...@gmail.com on 2012-04-18T12:41:22Z

What steps will reproduce the problem?

  1. In a console, create a list like a=[1,2,3]
    1. Type a.pop(
    2. Examine the help that pops up in the object inspector. What is the expected output? What do you see instead? Expect to see the full docstring. Some of the docstring is missing. Roll back to the revision f73ae696b3b4 to compare. Please use labels and text to provide additional information. Problem shows up at revision b3713a3dc120 . See additional comments on this code review: https://code.google.com/p/spyderlib/source/detail?r=b3713a3dc1201dcaba03d0dd0d3d537ef854d041 I think the behavior at revision f73ae696b3b4 was awesome. Just enough formatting to make the docstring look fancy without losing information. Might we consider just rolling back?

Original issue: http://code.google.com/p/spyderlib/issues/detail?id=997

@spyder-bot
Copy link
Collaborator Author

From ccordoba12 on 2012-04-18T11:09:31Z

I see exactly what you mean. I wasn't sure about removing the first line of the docstring because several python objects have annotations next to the signature, but in the ones I inspected they were unimportant. However most numpy/scipy objects only have the signature, so I decided to just remove it.

Instead of rolling back, what about removing the first line if and only if there is a signature in it? If there isn't, we could leave it as it is. This will solve the problem you are reporting but also will let us to avoid to show repeated info (in the definition and the docstring body), which was my intention.

It just would be a matter to improve the regexp we are using here https://code.google.com/p/spyderlib/source/browse/spyderlib/utils/dochelpers.py#120 or build a new one here https://code.google.com/p/spyderlib/source/browse/spyderlib/utils/dochelpers.py#89 Thanks a lot for testing. This is the kind of feedback I'm expecting to improve on my initial attempt and make the feature rock solid!

Labels: MS-v2.2

@spyder-bot
Copy link
Collaborator Author

From jed.lud...@gmail.com on 2012-04-18T11:50:46Z

I would avoid throwing away any text from the docstring. If you can implement the magic that can detect when the signature you are about to display in the Definition field is going to be an exact duplicate of the first line of the docstring then it's probably okay to eliminate just the signature from being displayed again. But that's about as far as I'd go.

@spyder-bot
Copy link
Collaborator Author

From pierre.raybaut on 2012-04-19T00:08:36Z

I fully agree with Jed on this one.

@spyder-bot
Copy link
Collaborator Author

From jed.lud...@gmail.com on 2013-01-14T10:32:13Z

@-ccordoba12: I still run into this issue from time to time. Have you thought more about how to address it? It seems like a more complex regular expression might do the trick, but I haven't looked into the code deed enough to know how to contribute yet. If you want me to take a crack at it let me know.

@spyder-bot
Copy link
Collaborator Author

From jed.lud...@gmail.com on 2013-01-15T08:07:54Z

@-ccordoba12: I've come up with a patch that accomplishes what you were suggesting in comment 1, and it meets the desire I expressed in comment 2. Take a look and see if you approve. If so, I can push the change.

Attachment: docstring.patch

@spyder-bot
Copy link
Collaborator Author

From jed.lud...@gmail.com on 2013-01-15T09:02:30Z

Actually, I like this patch a little bit better. I cleans up the extra signature line that shows up with numpy and scipy docs but it is a little less invasive with docstrings on standard Python functions.

Attachment: docsting_improved.patch

@spyder-bot
Copy link
Collaborator Author

From ccordoba12 on 2013-01-15T09:17:19Z

Thanks for taking the time to look at this issue and sorry for the huge delay in taking care of it.

Now to the point: What's the difference between the two patches? I only see these line changing, from:

title_and_argspec = text['title']+text['argspec']

in docstring.patch to

title_and_argspec = text['title']+text['argspec']+'\n'

in docsting_improved.patch

@spyder-bot
Copy link
Collaborator Author

From jed.lud...@gmail.com on 2013-01-15T09:22:58Z

The inclusion of the '\n' is the only difference. If that's not there, then some of the standard Python docstrings (look at 'float' for example) get broken up in a non-intuitive way.

@spyder-bot
Copy link
Collaborator Author

From jed.lud...@gmail.com on 2013-01-15T12:52:59Z

And one last attempt using regular expressions that allows for additional white space to be on the first line in addition to the title and argspec. Use this one if you feel like the previous two patches were too restrictive regarding potential extra white space that might be in the first line of the docstring.

Attachment: docstring_regex.patch

@spyder-bot
Copy link
Collaborator Author

From jed.lud...@gmail.com on 2013-01-23T19:57:49Z

@-ccordoba12: I've been running with the last patch at comment 9 for several days now, and it produces precisely the behavior you suggested in comment 1. I would go with it. If you want me to push it out I'm happy to do it!

@spyder-bot
Copy link
Collaborator Author

From ccordoba12 on 2013-01-23T20:19:03Z

Sorry for not reviewing this before. I prefer your patch in comment #6 because it's far more easier to read and understand than the regex you developed in comment #9.

I mentioned a regex in comment #1 because I thought this problem was introduced by a regex I changed while I was developing this feature.

I would only suggest to add a comment to explain the '\n' we talked about and that'll be all from my side :)

You can push it after that if you want.

Owner: jed.lud...@gmail.com

@spyder-bot
Copy link
Collaborator Author

From jed.lud...@gmail.com on 2013-01-24T05:48:06Z

This issue was updated by revision e60482436b08 .

Status: Fixed

@spyder-bot
Copy link
Collaborator Author

From jed.lud...@gmail.com on 2013-01-24T07:39:48Z

This issue was updated by revision 7b50c4b5ae37 .

@spyder-bot
Copy link
Collaborator Author

From ccordoba12 on 2013-01-24T16:17:21Z

But Jed, I don't see the '\n' you mentioned in comment #8 in the commits you pushed. I thought it was necessary to render correctly docstrings like the 'float' one

Why is not there?

@spyder-bot
Copy link
Collaborator Author

From jed.lud...@gmail.com on 2013-01-24T16:29:32Z

The final commit I applied was slightly different than the patch in comment 8. I wanted it to be a little more tolerant of extra white space on the first line, and I came up with a simpler way to do that without having to resort to the complicated regex of comment 11. To do that I made use of the first_line variable you define a little above, which is basically a representation of the first doc string line with all white space stripped, including the '\n'. When I used that approach there was no more need to include the '\n' in the matching expression.

@spyder-bot
Copy link
Collaborator Author

From ccordoba12 on 2013-01-24T16:45:30Z

Ahh, ok. Glad to hear the solution was a simpler one :)

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

No branches or pull requests

1 participant