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

SG incorrectly grabbing the description when a label is defined in docstring #232

Closed
choldgraf opened this issue Apr 28, 2017 · 9 comments · Fixed by #345
Closed

SG incorrectly grabbing the description when a label is defined in docstring #232

choldgraf opened this issue Apr 28, 2017 · 9 comments · Fixed by #345

Comments

@choldgraf
Copy link
Contributor

choldgraf commented Apr 28, 2017

I noticed that SG is incorrectly grabbing the description when you define a label in the docstring before the title. E.g.:

"""
.. my_label:

=======
Title
=======

etc etc.
"""

SG will correctly find title, but when it tries to pull the description it shows up as =====Title===== because it's splitting on the wrong line.

For example see my matplotlib (new) tutorials build here: http://predictablynoisy.com/matplotlib/tutorials/index.html

@Titan-C
Copy link
Member

Titan-C commented Apr 28, 2017 via email

@choldgraf
Copy link
Contributor Author

yeah - we had a debate about this with the MNE team a little while back, but there was pushback from people who didn't want to have such long labels. I don't think that we realized that this wasn't supported.

Kind of a bummer because that means we need to go back through many many matplotlib examples that already have a label in them and switch it to the long sphinx-gallery version :-/ that said, I can understand why you don't want to support this. In that case, it'd be helpful if there was an error any time the first line of the docstring wasn't part of a title, instead of it working but silently pulling the wrong text for description.

@choldgraf
Copy link
Contributor Author

One final thought on this: one reason we've had disagreements about using the sphinx gallery links etc is that they can be really, really long. If you're using something like a PEP checker that's forcing all of your lines to be <= 79 characters, this means that one sphinx gallery link can take up like 80% of the length of a line.

That said, I got all of the links etc working for matplotlib's docs, so it's not a dealbreaker. I just think it's worth considering. I'll close the issue tho if it's not something worth implementing! (I promise I'm not trying to be confrontational, I just don't like dangling issues that don't have anything actionable in them :-) )

@Titan-C
Copy link
Member

Titan-C commented May 1, 2017 via email

@choldgraf
Copy link
Contributor Author

I should say that right now I am coming from the perspective of a person trying to convert all of the matplotlib docs over to sphinx-gallery...so I am probably not representative of the doc situation in most packages. Just trying to share my experience thus far :D

At the very least, I think it's worth adding some kind of warning or callout in the docs for SG that says "note: you should not put anything before the title in the docstring" so that users don't think this is a bug like I did :)

@lesteve
Copy link
Member

lesteve commented May 9, 2017

Sorry I am just looking at this now ...

Just trying to share my experience thus far :D

This is super useful, thanks a lot for this, keep the comments coming our way !

In this particular case, it does feel like we way we find the title and description for the tooltip is too fragile.

Since you seem to say that the title is found correctly maybe we can just remove the title from the docstring and find the paragraph once the title has been removed. This would make the whole thing a little less brittle.

Side-note: I agree that parsing the rst would be a better way to do it, I don't know too much about sphinx and sphinx monkey-patching docutils certainly does not encourage me to look further into it ...

@choldgraf
Copy link
Contributor Author

Since you seem to say that the title is found correctly maybe we can just remove the title from the docstring and find the paragraph once the title has been removed. This would make the whole thing a little less brittle.

I'm a +1 for this, though also understand if the preferred behavior is to just raise an error if it finds anything in the docstring above the title. The main thing to me is that it shouldn't silently do the wrong thing :-)

@larsoner
Copy link
Contributor

larsoner commented Feb 9, 2018

@choldgraf I think this has been fixed in some PR (one of mine I think?), too

@choldgraf
Copy link
Contributor Author

yep! thanks for squashing these issues

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 a pull request may close this issue.

4 participants