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鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

Deduplicate attribute docs of ContourSet and its derived classes #17799

Merged
merged 1 commit into from
Jul 7, 2020

Conversation

timhoffm
Copy link
Member

PR Summary

I'm not 100% sure if we feel it's ok to build custom interpolation decorators, but it works 馃槃 .

@tacaswell tacaswell added this to the v3.4.0 milestone Jul 1, 2020
@tacaswell
Copy link
Member

I'm leaving an approval, but want a second set of eyes on this.

On one hand this makes no difference to the interactively accessed docs and the built docs, reduces the LOC we have floating around and means that we can be sure that these docstrings all stay in sync, but it comes at the cost of making the docstrings being much less clear when you are reading through the docs (which is what I spend most of my time doing so I am maybe over-sensitive to that).

I think that this is a net positive, but want to make sure someone else agrees.

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.

If one looks up the docs using e.g. ?contour.ContourSet in a jupyter notebook do the docs still properly render as opposed to having %(contour_set_attributes)s in them? If they do properly render, then I'm 馃憤 for this change

@jklymak
Copy link
Member

jklymak commented Jul 3, 2020

Don鈥檛 we already do docstring interpolation other ways? Not a fan of a new mechanism. Also it鈥檚 really helpful to have consistent prefixes for the docstring interpolation variables so we can easily search for them.

@QuLogic
Copy link
Member

QuLogic commented Jul 3, 2020

The other way is to put the replacements on a global Substitution instance; is there a reason these can't go on there?

@timhoffm
Copy link
Member Author

timhoffm commented Jul 4, 2020

Changed to use the global Substitution instance docstring.interpd.

@QuLogic QuLogic merged commit 1b12cd5 into matplotlib:master Jul 7, 2020
@timhoffm timhoffm deleted the doc-tricontourset branch July 7, 2020 08:03
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.

None yet

5 participants