Resolves #26421 Added an example for fig comparison decorator#26538
Resolves #26421 Added an example for fig comparison decorator#26538story645 merged 28 commits intomatplotlib:mainfrom
Conversation
Added an example for figure comparison/
Update testing.rst
There was a problem hiding this comment.
Thank you for opening your first PR into Matplotlib!
If you have not heard from us in a week or so, please leave a new comment below and that should bring it to our attention. Most of our reviewers are volunteers and sometimes things fall through the cracks.
You can also join us on gitter for real-time discussion.
For details on testing, writing docs, and our review process, please see the developer guide
We strive to be a welcoming and open project. Please follow our Code of Conduct.
Fixed trailing spaces Co-authored-by: Kyle Sunden <[email protected]>
|
@ksunden thank you! |
story645
left a comment
There was a problem hiding this comment.
Thanks for tackling this! I think it's a good start but the example needs to show the use of both fig_ref and fig_test
doc/devel/testing.rst
Outdated
| then collect the drawn results and compare them. For example, this test draws | ||
| the same circle on the figures with two different methods:: |
There was a problem hiding this comment.
maybe me more explicit about the two methods (drawing via adding circle patch vs drawing via plotting a circle)? And change the name of the test method to whichever method you're testing here?
There was a problem hiding this comment.
That makes sense!
There was a problem hiding this comment.
Can you elaborate the second part? I am not quite sure what you mean. Do you mean change the name of the test method in the code?
There was a problem hiding this comment.
Do you mean change the name of the test method in the code?
Exactly! Something like test_parametric_circle_plot
There was a problem hiding this comment.
Okay! I'll edit it.
Made changes to add fig_test and made the documentation more cohesive.
Removed import pytest and added the trailing space.
story645
left a comment
There was a problem hiding this comment.
Slight wording change and preference for more explicit test name, but this can still be merged if you don't have time to make those changes-I'll just give it a day or so before merging to give you time to respond.
|
Looks like there remains some trailing whitespace, probably in the paragraphs (Github highlights it when it was on its own line, so that is all I saw earlier, but CI indicates it is still there) |
Co-authored-by: hannah <[email protected]>
I'll try to edit it but I am good with you merging it. :) |
How can I remove all of those? |
Tried to remove most of the trailing spaces and changed the name of the test.
Have you installed the pre-commit hooks? They should do that automatically. https://matplotlib.org/devdocs/devel/development_setup.html#install-pre-commit-hooks-optional |
I did not have them but I have installed them now. Thank you! Do I have to make new commit to remove the trailing spaces? |
Kinda yes - you add and commit the file after the pre-commit hook runs & only push up once clean. Also can you rebase the file or would you like help or for us to do it on merge? |
I tried to commit to the file but it failed the don't commit to main branch test. I have never used pre-commit before but I tried to push the changes but I was not able to. It would be great if you could help me with or instruct how to do it. Thank you. |
That's not a big deal because this is a relatively small document change, but in the future please make a feature branch - and I really do hope you do more PRs cause I think you wrote really well here. So to push ignoring a pre-commit, you can add in a --no-verify flag to the commit where the only failing test is the no-commit-to-main: git commit doc/devel/testing.rst -m 'Resolving pre-commit-hook changes' --no-verifyThen to do a rebase, which editor are you using? |
I'll keep in mind to create a feature branch next time. Thank you and I'd love to contribute and do more PRs. I got to learn a lot from this one. Thank you. I ran the command worked and its working well. I use usually use VSCode as an editor. |
|
Here's a really good tutorial for git rebase in vscode https://www.youtube.com/watch?v=3o_01F04bZ4 and if it seems too much right now just let me know cause looks like I can squash merge this |
I saw the video and it really helped but I am afraid I'll need some practice in rebasing, so it would be great if you squash merge this. Thank you! |
Co-authored-by: hannah <[email protected]>
Co-authored-by: hannah <[email protected]>
Co-authored-by: hannah <[email protected]>
Co-authored-by: hannah <[email protected]>
Co-authored-by: hannah <[email protected]>
Co-authored-by: hannah <[email protected]>
Co-authored-by: hannah <[email protected]>
Should I add an import statement for pytest too? Or is it given since it was written in doc. |
Co-authored-by: hannah <[email protected]>
it's given since it was written in doc. |
…ple for fig comparison decorator
|
Something went wrong ... Please have a look at my logs. It seems that the branch you are trying to backport to does not exist. |
|
Thank you so much for seeing this PR through, congrats on your first merged PR, and we hope to see you again! |
Thank you for helping me through it. I got to learn a lot and I look forward to contributing again!! |
…ple for fig comparison decorator
…t-of-pr-26538-on-v3.7.x Backport PR matplotlib#26538 on branch v3.7.x (Resolves matplotlib#26421 Added an example for fig comparison decorator)
PR summary
Added an example of figure comparison decorator check_figures_equal() in testing doc. @story645 please review and let me know if I made some mistakes.
PR checklist