Movatterモバイル変換

[0]ホーム
URL:

Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

 Sign up
Appearance settings

plot directive: caption-option#18426

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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.

Already on GitHub?Sign in to your account

Merged
QuLogic merged 6 commits intomatplotlib:masterfromulijh:plot-directive-caption-option
Sep 12, 2020
Merged

plot directive: caption-option#18426

QuLogic merged 6 commits intomatplotlib:masterfromulijh:plot-directive-caption-option
Sep 12, 2020

Conversation

ulijh
Copy link
Contributor

PR Summary

This PR adds an option :caption: to the plot directive. It partially solves#9346.

  • The new option enforces a caption when a single figure is generated.

  • The option take precedence over the contents when a separate file containing the code is used.

  • Tested and documented in the module's doc string.

PR Checklist

  • Has pytest style unit tests (andpytest passes).
  • IsFlake 8 compliant (runflake8 on changed files to check).
  • New features are documented, with examples if plot related.
  • Documentation is sphinx and numpydoc compliant (the docs shouldbuild without error).
  • Conforms to Matplotlib style conventions (installflake8-docstrings andpydocstyle<4 and runflake8 --docstring-convention=all).
  • New features have an entry indoc/users/next_whats_new/ (follow instructions in README.rst there).
  • API changes documented indoc/api/next_api_changes/ (follow instructions in README.rst there).

This new option allows the user to add a caption to plots with theplotting code given in the directive's contents.* The option :caption: enforces a caption.* The option take precedence over the contents.* Tested and documented in the modul's doc string.* Note in next_whats_new
@tacaswelltacaswell added this to thev3.4.0 milestoneSep 7, 2020
Copy link
Member

@timhoffmtimhoffm left a comment

Choose a reason for hiding this comment

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

Modulo raising on ambiguity.

@ulijh
Copy link
ContributorAuthor

@tacaswell@timhoffm after I submitted I found out there was another PR#15304 doing the same thing. Could you have a look and check which one should be taken forward? Thanks a bunch.

@jklymak
Copy link
Member

@ulijh, thanks for finding that. I would tend to say luck of the draw on this one. The original PR author probably needed to nag us a bit - if you are hung ho to see this through lets stick w/ this PR.

@ulijh
Copy link
ContributorAuthor

I've just changed this to raise if the caption is given twice.

Two things I could need some help with:

  1. How to get the rst source line into the error message, and

  2. how to test this properly. Creating a whole new tinypages for this seems a bit over the top. May be the tests inSphinx extension: support captions in inline plots. #15304 can help with that? If so, may be we should merge this PRs.@bcbnz what do you think?

Copy link
Member

@QuLogicQuLogic left a comment

Choose a reason for hiding this comment

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

Some small typos.

I'm not sure we have any way to test failure cases for the Sphinx extension.

@QuLogic
Copy link
Member

QuLogic commentedSep 9, 2020•
edited
Loading

You can add proper context usingself.error.However, the way we set up things to support both the class-basedDirective and standalone function means you don't haveself.

To support both still, I think you'd have to add atry/except Exception as e around here:

defrun(self):
"""Run the plot directive."""
returnrun(self.arguments,self.content,self.options,
self.state_machine,self.state,self.lineno)

and then callself.error(str(e))

ulijhand others added4 commitsSeptember 9, 2020 07:38
Co-authored-by: Elliott Sales de Andrade <quantum.analyst@gmail.com>
Fix typo in sphinx test document.Co-authored-by: Elliott Sales de Andrade <quantum.analyst@gmail.com>
Fix typo in comment.Co-authored-by: Elliott Sales de Andrade <quantum.analyst@gmail.com>
@ulijh
Copy link
ContributorAuthor

Hey guys, the context is added when the error is raised. From my point of view this seems OK to merge. Thanks to the reviewers!

@QuLogicQuLogic merged commit33affe1 intomatplotlib:masterSep 12, 2020
@QuLogic
Copy link
Member

Thanks@ulijh! Congratulations on your first PR to Matplotlib 🎉 We hope to hear from you again.

@ulijhulijh deleted the plot-directive-caption-option branchSeptember 16, 2020 09:25
@ulijh
Copy link
ContributorAuthor

Thanks for merging! :-) I'm very happy to have contributed (a little bit) to this great project! Thanks to all who invest their time in it!

Sign up for freeto join this conversation on GitHub. Already have an account?Sign in to comment
Reviewers

@tacaswelltacaswelltacaswell approved these changes

@QuLogicQuLogicQuLogic left review comments

@timhoffmtimhoffmtimhoffm approved these changes

Assignees
No one assigned
Projects
None yet
Milestone
v3.4.0
Development

Successfully merging this pull request may close these issues.

5 participants
@ulijh@jklymak@QuLogic@tacaswell@timhoffm
[8]ページ先頭
©2009-2025 Movatter.jp