Skip to content

Update examples for axisgrid1 #9447

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 1 commit into from
Oct 19, 2017
Merged

Update examples for axisgrid1 #9447

merged 1 commit into from
Oct 19, 2017

Conversation

divyam3897
Copy link
Contributor

Add description for examples for axisgrid1/demo_axis_*
Extension to #8921

@story645 story645 changed the title Updaye examples for axisgrid1 Update examples for axisgrid1 Oct 16, 2017
Copy link
Member

@jklymak jklymak left a comment

Choose a reason for hiding this comment

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

Thanks again for these.

I'd drop "Example" from all of these. They are self-evidently examples.

I also think we should stop calling these "Demo X Y", etc. Its also evident they are demos. More clever titles but still short titles would be helpful.

@dstansby dstansby added this to the v2.1.0-doc milestone Oct 17, 2017
@@ -3,6 +3,7 @@
Demo Axes Divider
=================

Example using Axes divider to calculate location of axes and create a divider for them using exisiting axes instances.
Copy link
Member

Choose a reason for hiding this comment

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

This line isn't pep8: it's too long.

You can probably configure your text editor to show you the lines that don't follow pep8 automatically.

@@ -3,6 +3,8 @@
Demo Axes Divider
=================

Example using Axes divider to calculate location of axes
Copy link
Contributor

Choose a reason for hiding this comment

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

I think this needs to be one line for sphinx-gallery to parse it properly. I could be wrong though... @NelleV do you know?

Copy link
Member

Choose a reason for hiding this comment

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

Nope, it doesn't.

@jklymak
Copy link
Member

jklymak commented Oct 19, 2017

My concerns still stand. “Example demonstrating....” is (doubly!) redundant. “Demo Axes Divider” should just be “Axes Divider” etc.

@choldgraf
Copy link
Contributor

choldgraf commented Oct 19, 2017

This looks good to me - agree that it'd be better if "demo" or "example" weren't in there, but IMO we can handle those things in a subsequent blanket PR that makes this fix for all examples...I get the feeling that there are many places where this problem of redundancy is in the docs. Shall I open an issue about it?

@divyam3897
Copy link
Contributor Author

@choldgraf true, there are many examples having that redundancy, it should be uniform and no example should have them in that case. I can remove it in this PR only or a separate PR later for all of them whatever seems right?

@jklymak
Copy link
Member

jklymak commented Oct 19, 2017

@divyam3897 You can remove the redundancy from the caption text, and we can worry about the redundancy in the titles later is what I think @choldgraf means. Thanks!

@NelleV
Copy link
Member

NelleV commented Oct 19, 2017

We have a lot of examples, so I wouldn't attempt to do this all at once.

What is important is to have meaningful titles and description of the content of the example. "Example demonstrating Hbox Divider to arrange subplots." doesn't really describe what Hbox divider does or how it helps arrange subplots for example.

I'm fine merging this as is, as it is an improvement on the current state of the gallery (thanks for the patch!), but in general, I'd rather have less examples or changes, but more meaningful one.

@divyam3897
Copy link
Contributor Author

@jklymak @NelleV @choldgraf I made the changes, let me know if I can improve them more 👍

Copy link
Member

@dstansby dstansby left a comment

Choose a reason for hiding this comment

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

👍

@jklymak jklymak merged commit 32bd143 into matplotlib:master Oct 19, 2017
@jklymak
Copy link
Member

jklymak commented Oct 19, 2017

Thanks a lot @divyam3897

So just for reference, I think these descriptions at the top of examples can be as long as we like? If so, I don't think it hurts future ones to be a bit more informative if warranted (without them becoming a full-on tutorial)

@divyam3897
Copy link
Contributor Author

@jklymak so a 2-3 lines of description would be better?

@divyam3897 divyam3897 deleted the updateDescriptions branch October 19, 2017 19:36
@jklymak
Copy link
Member

jklymak commented Oct 19, 2017

@divyam3897 Not sure - I'm relatively new too, but I don't think it would hurt. If you are going through a few more of these, and the example is complicated, feel free to use a couple of lines. We can always edit it back.

@choldgraf
Copy link
Contributor

choldgraf commented Oct 19, 2017 via email

@QuLogic
Copy link
Member

QuLogic commented Oct 20, 2017

@tacaswell not backporting automatically?

@tacaswell
Copy link
Member

@meeseeksdev backport to v2.1.0-doc

lumberbot-app bot pushed a commit that referenced this pull request Oct 21, 2017
tacaswell added a commit that referenced this pull request Oct 21, 2017
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.

7 participants