Skip to content

DOC: small-doc-improvements1 #11161

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

Conversation

ImportanceOfBeingErnest
Copy link
Member

@ImportanceOfBeingErnest ImportanceOfBeingErnest commented May 2, 2018

PR Summary

PR Checklist

  • Code is PEP 8 compliant
  • Documentation is sphinx and numpydoc compliant

@timhoffm
Copy link
Member

timhoffm commented May 2, 2018

As of numpydoc (section on Returns) simple types or name : type combinations are supported as elements. It's a bug in numpydoc that :class:`~.container.ErrorbarContainer` does not work (numpy/numpydoc#72).

For most of the cases given here, I'd actually rather not provide a name because that has no additional meaning. So my preferred choice would be to get that fixed in numpydoc. If you really want to work around this within matplotlib, use a name conforming with the variable naming convention, e.g.

  • c : ErrorbarContainer
  • container : ErrorbarContainer
  • errorbar_container : ErrorbarContainer

(for me preferrably the first one since the name does not convey any additional semantics and short is less distracting).

@jklymak
Copy link
Member

jklymak commented May 2, 2018

This and #11160 are partially doing the same thing.

@ImportanceOfBeingErnest
Copy link
Member Author

ImportanceOfBeingErnest commented May 2, 2018

What does the letter c signify? Given some return type MyCustomReturnType, which letter from the alphabet would I choose?

@timhoffm
Copy link
Member

timhoffm commented May 2, 2018

Just shortening to the first letter: errorbar_container, container, c. Could also be ec. A guide line for me is "What's a meaningful simple word you would use to describe the returned object?". Is is a

  • container - ok --> c
  • errorbar - no
  • errorbar_container - ok but a bit longer --> ec

You don't have to be very verbose here if you would just repeat the type.

Note: This is just my personal preference. There are no objective rules how to to it.

Copy link
Member

@timhoffm timhoffm left a comment

Choose a reason for hiding this comment

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

Accepting as a temporary workaround for numpy/numpydoc#72. Once the issue in numpydoc gets resolved, we should go back to leaving out unmeaningful names for return values and just specify the type.

@ImportanceOfBeingErnest
Copy link
Member Author

I'm much more in favor of actual words or names being used. Otherwise people could spend a good amount of time searching for what c is.
Clearly any solution that does provide a link is advantageous compared to one without links. And who knows when the numpydoc bug would be fixed?!

Something is really strange about the checks. Previously I got an error from some image test failing in appveyor/ci; now I got test coverage being diminished. This wouldn't have anything to do with this PR though, would it?

@@ -2148,7 +2151,9 @@ def barh(self, y, width, height=0.8, left=None, *, align="center",

- scalar: symmetric +/- values for all bars
- shape(N,): symmetric +/- values for each bar
- shape(2,N): separate + and - values for each bar
- shape(2,N): Separate - and + values for each bar. First row
Copy link
Member

Choose a reason for hiding this comment

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

I don't think this sentence will be very clear to new users. Could you add an Example numpydoc section with a short example example demonstrating the clarification?

Copy link
Member Author

Choose a reason for hiding this comment

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

I added a link to this example.

@ImportanceOfBeingErnest ImportanceOfBeingErnest force-pushed the small-doc-improvements1 branch 2 times, most recently from 618c8bc to 95da955 Compare May 2, 2018 21:17
@ImportanceOfBeingErnest
Copy link
Member Author

I also took the freedom to set a new css rule for listed items in parameter sections of the docs. Previously, the listed text had a different fontsize than the explaining text outside of a list.
image

This is now changed to have the same fontsize.

image

@timhoffm timhoffm added this to the v2.2.3 milestone May 2, 2018
@jklymak jklymak merged commit 2f3e81b into matplotlib:master May 2, 2018
lumberbot-app bot pushed a commit that referenced this pull request May 2, 2018
@ImportanceOfBeingErnest ImportanceOfBeingErnest deleted the small-doc-improvements1 branch May 2, 2018 23:01
jklymak added a commit that referenced this pull request May 2, 2018
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.

4 participants