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

Update examples for axisgrid1 #9447

Merged
merged 1 commit into from Oct 19, 2017

Conversation

Projects
None yet
7 participants
@divyam3897
Copy link
Contributor

commented Oct 16, 2017

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

@jklymak
Copy link
Contributor

left a comment

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.

This comment has been minimized.

Copy link
@NelleV

NelleV Oct 19, 2017

Member

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

This comment has been minimized.

Copy link
@choldgraf

choldgraf Oct 19, 2017

Contributor

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

This comment has been minimized.

Copy link
@NelleV

NelleV Oct 19, 2017

Member

Nope, it doesn't.

@jklymak

This comment has been minimized.

Copy link
Contributor

commented Oct 19, 2017

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

@choldgraf

This comment has been minimized.

Copy link
Contributor

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

This comment has been minimized.

Copy link
Contributor Author

commented Oct 19, 2017

@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

This comment has been minimized.

Copy link
Contributor

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

This comment has been minimized.

Copy link
Member

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

This comment has been minimized.

Copy link
Contributor Author

commented Oct 19, 2017

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

@dstansby
Copy link
Contributor

left a comment

👍

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

2 of 5 checks passed

continuous-integration/appveyor/pr Waiting for AppVeyor build to complete
Details
continuous-integration/travis-ci/pr The Travis CI build is in progress
Details
lgtm analysis: Python Running analyses for revisions
Details
ci/circleci: docs-python27 Your tests passed on CircleCI!
Details
ci/circleci: docs-python35 Your tests passed on CircleCI!
Details
@jklymak

This comment has been minimized.

Copy link
Contributor

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

This comment has been minimized.

Copy link
Contributor Author

commented Oct 19, 2017

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

@divyam3897 divyam3897 deleted the divyam3897:updateDescriptions branch Oct 19, 2017

@jklymak

This comment has been minimized.

Copy link
Contributor

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

This comment has been minimized.

Copy link
Contributor

commented Oct 19, 2017

@QuLogic

This comment has been minimized.

Copy link
Member

commented Oct 20, 2017

@tacaswell not backporting automatically?

@tacaswell

This comment has been minimized.

Copy link
Member

commented Oct 21, 2017

@meeseeksdev backport to v2.1.0-doc

meeseeksdev 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
You can’t perform that action at this time.