Testing MPL with a notebook in binder#4544
Merged
1ucian0 merged 22 commits intoQiskit:masterfrom Jun 22, 2020
Merged
Conversation
Member
Author
|
In fcd2830 I added a reference to the test generating the image file:
|
Member
Author
|
In f47b0f3 I added a "diff".
|
Member
Author
|
In a3a994f I added folding results. When passing (default is folded): When failing (default is showing): When the reference is not found: |
Member
|
This is awesome. Do you know if the diff occurs pixel by pixel? For example if I slightly shift an image to the right it’s still the same image but all the pixels are different compared to reference.
… On Jun 10, 2020, at 2:22 PM, Luciano Bello ***@***.***> wrote:
In a3a994f I added folding results.
When passing (default is folded):
When failing (default is showing):
When the reference is not found:
—
You are receiving this because your review was requested.
Reply to this email directly, view it on GitHub, or unsubscribe.
|
Member
Author
It's a pixel by pixel comparison. Unmatching pixels are going to be highlighted in the diff. If there is a shift, the reference image should be refreshed. |
Member
Author
|
I added autoexecution. The result can be seen here: |
This was referenced Jun 11, 2020
mtreinish
approved these changes
Jun 22, 2020
This was referenced Jun 23, 2020
Merged
mtreinish
added a commit
to mtreinish/qiskit-core
that referenced
this pull request
Jul 9, 2020
In Qiskit#4544 a github action workflow was added to attempt to automate leaving a comment on PRs that made changes to the matplotlib with a link to a jupyter environment running in binder that will show a visual diff of the changes being made. But this workflow can't work for PRs opened from forks (which is most of them) because the permissions for the github actions workflow only allows reading from the github api [1] and will not be able to leave comments. This is done for obvious security reasons because a job triggered by an external fork can run arbitrary code so you don't want to give any elevated permissions until the code has been verified. Since this approach will never be viable using github actions (or a similar ci platform) this commit removes the job.
1ucian0
pushed a commit
that referenced
this pull request
Jul 9, 2020
In #4544 a github action workflow was added to attempt to automate leaving a comment on PRs that made changes to the matplotlib with a link to a jupyter environment running in binder that will show a visual diff of the changes being made. But this workflow can't work for PRs opened from forks (which is most of them) because the permissions for the github actions workflow only allows reading from the github api [1] and will not be able to leave comments. This is done for obvious security reasons because a job triggered by an external fork can run arbitrary code so you don't want to give any elevated permissions until the code has been verified. Since this approach will never be viable using github actions (or a similar ci platform) this commit removes the job.
faisaldebouni
pushed a commit
to faisaldebouni/qiskit-terra
that referenced
this pull request
Aug 5, 2020
faisaldebouni
pushed a commit
to faisaldebouni/qiskit-terra
that referenced
this pull request
Aug 5, 2020
In Qiskit#4544 a github action workflow was added to attempt to automate leaving a comment on PRs that made changes to the matplotlib with a link to a jupyter environment running in binder that will show a visual diff of the changes being made. But this workflow can't work for PRs opened from forks (which is most of them) because the permissions for the github actions workflow only allows reading from the github api [1] and will not be able to leave comments. This is done for obvious security reasons because a job triggered by an external fork can run arbitrary code so you don't want to give any elevated permissions until the code has been verified. Since this approach will never be viable using github actions (or a similar ci platform) this commit removes the job.
This was referenced Sep 4, 2020
Merged
Contributor
|
wow, this so cool. I always wondered if github would be able to one day report diffs on images in a more human readable way... you just did it 👏 |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
4 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes #4533
Fixes #2968
Summary
In the effort to improve the MPL drawer testing, this PR offers a way to do "visual" test. Here is the workflow:
qiskit/visualization/matplotlib.py
3. The link to the binder, triggers an online notebook that needs to be executed:
4. Each image is compared with a reference manually.Details of the PR
test/ipynb/mpl_tester.ipynbThe notebook that is going to be run in Binder. It was cleaned up withnb-cleantest/ipynb/mpl/test_circuit_matplotlib_drawer.pyThe notebook runs this tests. They have no assert as their goal is to create the images. And check that there are no exceptions..gitignoreThe images from the previous point are stored intest/ipynb/mpl/*.png, so they are ignored.test/ipynb/mpl/references/Where the reference images are stored.test/ipynb/results.pyIt is run in the second cell of the notebook for showing the results..github/workflows/binder_mpl.ymlIt's the action to add the comment whenqiskit/visualization/matplotlib.pyis modified.postBuildBinder runs this file after the build, sincematplotlibis not installed automaticallyFurther work
Does not show the results when there are no changesHave a easy way to see image differences (something like substraction)The binder to this PR can be found here https://mybinder.org/v2/gh/1ucian0/qiskit-terra/binder?filepath=test/ipynb/mpl_tester.ipynb