Skip to content

Add sentence to textprops tutorial mentioning mathtext rcParams #8853

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

Merged
merged 4 commits into from
Jul 11, 2017

Conversation

geib7592
Copy link
Contributor

@geib7592 geib7592 commented Jul 9, 2017

PR Summary

Addresses issue #8702.

Minor change to the text_props tutorial to point out that the math font needs to be set with the mathtext rcParams. I also added a label to the math tutorial so I could link to it.

I'm a new contributor, and this is my first PR. Please let me know if I've done anything incorrectly, and thanks for your patience! If all goes well, I'd like to get more involved!

PR Checklist

  • Has Pytest style unit tests
  • Code is PEP 8 compliant
  • New features are documented, with examples if plot related
  • Documentation is sphinx and numpydoc compliant
  • Added an entry to doc/users/whats_new.rst if major new feature
  • Documented in doc/api/api_changes.rst if API changed in a backward-incompatible way

@tacaswell
Copy link
Member

Thanks for the PR! 👍 to the content, but sphinx is not happy:

Warning, treated as error:
/home/travis/build/matplotlib/matplotlib/doc/tutorials/text/text_props.rst:162:undefined label: mathtext_fonts (if the link has no caption the label must precede a section header)

and I am not sure why....

@tacaswell
Copy link
Member

I wonder if this is an order of compilation issue with sphinx gallery? attn @choldgraf ?

@tacaswell tacaswell added this to the 2.1 (next point release) milestone Jul 9, 2017
@choldgraf
Copy link
Contributor

huh....that's weird. It looks like the label is defined properly. Do the generated RST files all look correct? (inside of tutorials?

@geib7592
Copy link
Contributor Author

geib7592 commented Jul 9, 2017

Oh yeah, it looks likes the generated mathtext RST is putting a bunch of stuff in a code block that shouldn't be (including the definition of the label I'm trying to reference.)

@choldgraf
Copy link
Contributor

maybe it's an indentation problem then - double check that everything is indented properly!

@geib7592
Copy link
Contributor Author

Comparing mathtext.py and text_props.py, I noticed that mathtext.py didn't start the comment section with a line of ####. I added that, and it compiled further. Then there was a missing # on line 36. Once those were in, the documentation compiled.

Also, I had to remove some unused footnotes in a in order to get the documentation to compile. Not sure if you guys got that error too. I'm running Sphinx 1.6.2.

@tacaswell
Copy link
Member

I think you are working from and old checkout, those references were already removed in 3649684

@tacaswell
Copy link
Member

This is a candidate for squash-merging.

@geib7592 If you are comfortable doing so, it might be worth rebasing this PR.

Thanks for working through the sphinx issues 😄

@geib7592
Copy link
Contributor Author

Darnit. Well, I'm learning as I go. So I attempted to rebase, and now this PR contains commits that are unrelated to what I changed. Is this fixable, or should I just start over?

@tacaswell
Copy link
Member

git branch backup  # create a branch where you are just to be safe!
git rebase -i  origin/master  # follow the directions to drop the commits you do not want
git push --force your_fork
git branch -d backup  # if you are happy

@tacaswell
Copy link
Member

And creating git-snarls is something that happens occasionally to everyone (I am currently at scipy watching some tutorial presenters sort out some git issues 😎 )

@choldgraf
Copy link
Contributor

@tacaswell is there any resource we can point contributors to that explains more clearly how to rebase? If not I can write something up...rebasing is pretty confusing the first several times it's done...

@tacaswell
Copy link
Member

@choldgraf see #8398 We had a nice write up (to be fair I am biased because I wrote it) that some how got deleted.

@choldgraf
Copy link
Contributor

ah cool, that is v useful!

I had a quick comment on that text but I'll add it in the issue you opened.

@tacaswell
Copy link
Member

@geib7592 I think you missed the force push step (and did a git merge origin branch because the CLI suggested you do that) ;)

geib7592 added 2 commits July 10, 2017 20:49
…as adding the line of ### and then adding a # on one of the lines that was missing it
@geib7592
Copy link
Contributor Author

Thanks, it looked like I was digging myself deeper into the abyss. But I think it looks okay now.

@choldgraf
Copy link
Contributor

choldgraf commented Jul 11, 2017

Little known fact, when Nietzsche said

if you stare into the abyss, the abyss stares back at you

he was actually talking about rebasing off of a stale git branch

@@ -143,7 +143,9 @@
# Default Font
# ==============
#
# The base default font is controlled by a set of rcParams:
# The base default font is controlled by a set of rcParams. To set the font
# for mathematical expressions, use the rcParams begining with ``mathtext``
Copy link
Member

Choose a reason for hiding this comment

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

beginning

@tacaswell
Copy link
Member

👍 You are now through the worst of the git learning curve. http://tom.preston-werner.com/2009/05/19/the-git-parable.html is my favorite reference for this sort of thing.

@geib7592 geib7592 deleted the doc_textprops branch July 13, 2017 03:02
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants