Skip to content

WIP: Refactor test_basemap_polar to use mpl_image_compare and track png files in dvc #1071

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.

Sign up for GitHub

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

Jump to bottom
Closed
wants to merge 26 commits into from
Closed

WIP: Refactor test_basemap_polar to use mpl_image_compare and track png files in dvc #1071

wants to merge 26 commits into from

Conversation

seisman
Copy link
Member

@seisman seisman commented Mar 17, 2021

Testing #1036.

Reminders

  • Run make format and make check to make sure the code follows the style guide.
  • Add tests for new features or tests that would have caught the bug that you're fixing.
  • Add new public functions/methods/classes to doc/api/index.rst.
  • Write detailed docstrings for all functions/methods.
  • If adding new functionality, add an example to docstrings or tutorials.

Slash Commands

You can write slash commands (/command) in the first line of a comment to perform
specific operations. Supported slash commands are:

  • /format: automatically format and lint the code
  • /test-gmt-dev: run full tests on the latest GMT development version

weiji14 and others added 24 commits March 11, 2021 23:19
@weiji14
Initialize data version control
48fb2d9 
Adding dvc package to environment.yml and
running `dvc init` to get the barebones
.dvcignore, .dvc/config & .dvc/.gitignore files.
@weiji14
Set dvc remote as https://dagshub.com/GenericMappingTools/pygmt.dvc
4999875
@weiji14
Temporarily installing dvc using pip instead of conda to make CI work
9b61c77
@weiji14
Refactor test_logo to use mpl_image_compare and track png files in dvc
0c35dff
@weiji14
Add dvc install and dvc pull as a step in ci_tests.yaml to pull in data
567c967
@weiji14
Merge branch 'master' into data_version_control
7e0940c
@weiji14
List files in pygmt directory to see what happens after dvc pull
4833466
@weiji14
Do dvc pull before pip install otherwise test PNGs aren't there
f0ab167
@weiji14
Merge branch 'master' into data_version_control
f5e25fe
@weiji14
First draft of instructions for using dvc to store baseline images
6bd7ba9
@weiji14
Instruct to do git push first and then dvc push
e30c708 
Technically the order shouldn't matter, but most
tutorials seem to use `git push` first so follow that.
@weiji14
Merge branch 'master' into data_version_control
df1ab56
@weiji14
New checklist item for maintainers to get added to DAGsHub dvc remote
3208519
@weiji14 @seisman
Move pygmt/tests/baseline/.gitignore to top-level
2bd88c8 
Co-Authored-By: Dongdong Tian <[email protected]>
@weiji14 @seisman
Just use dvc push without setting --remote upstream
93f6d6e 
Co-Authored-By: Dongdong Tian <[email protected]>
@weiji14 @seisman
Clarify that git rm -r --cached only needs to run during migration
1f06f9a 
Co-Authored-By: Dongdong Tian <[email protected]>
@weiji14
Try installing dvc from conda again now that there is a Py3.9 package
e36fd28
@weiji14
Merge branch 'master' into data_version_control
f34bb09
@weiji14
Install dvc and do dvc pull on GMT dev tests too
f3aa3c5
@weiji14
Refactor test_logo tests to be simpler and more unit-test like
af79eef
@weiji14
Mention dvc status command to see which files need staging
5860a72
@weiji14
Use images for logo created using GMT 6.1.1
c37bdff
@weiji14
List only files under pygmt/tests/baseline
393773b
@seisman
Refactor test_basemap_polar to use mpl_image_compare and track png fi…
901733c 
…les in dvc
@seisman seisman force-pushed the fix-test-basemap-polar branch from 28fa478 to 901733c Compare March 18, 2021 02:30
@seisman seisman mentioned this pull request Mar 18, 2021
Merged
5 tasks
@weiji14
Copy link
Member

weiji14 commented Mar 18, 2021 •
edited
Loading

I just added a new image to my testing branch (commit b8bdb7c in #1071). However, I can't preview the image in DAGsHub: https://dagshub.com/GenericMappingTools/pygmt/src/fix-test-basemap-polar/pygmt/tests/baseline

Did you do dvc push successfully? Maybe try the sync button at the top of the page?

@seisman
Copy link
Member Author

seisman commented Mar 18, 2021 •
edited
Loading

Did you do dvc push?

Yes, I already did the dvc push, and it said one file is pushed. So the CI test should work (but it will fail because I generated the image using GMT 6.2.0).

Maybe try the sync button at the top of the page?

I'm afraid we have to push the sync button to preview the image. Not sure if our collaborators can also push the button.

@seisman
Copy link
Member Author

seisman commented Mar 18, 2021

Did you do dvc push?

Yes, I already did the dvc push, and it said one file is pushed. So the CI test should work (but it will fail because I generated the image using GMT 6.2.0).

The image is correctly downloaded using dvc pull. The tests all pass, perhaps there are no changes for this image between GMT 6.1.1 and GMT 6.2.0

Maybe try the sync button at the top of the page?

I'm afraid we have to push the sync button to preview the image. Not sure if our collaborators can also push the button.

Did you push the sync button? I don't, but now I can preview the image (about 10 minutes after I run the dvc push command):

image

@weiji14
Copy link
Member

weiji14 commented Mar 18, 2021

Did you push the sync button? I don't, but now I can preview the image (about 10 minutes after I run the dvc push command):

Yeah I pushed it. I thought you did but didn't see anything when I refreshed the page, so I just pressed it...

Maybe try the sync button at the top of the page?

I'm afraid we have to push the sync button to preview the image. Not sure if our collaborators can also push the button.

They should be able to push the button, that DAGsHub repo is just a clone of the GitHub repo so shouldn't have any permissions issue (all public anyway).

Another way I've thought about to preview the image is to use cml-publish at https://github.com/iterative/setup-cml to publish the image diff as a GitHub comment in the PR. It's originally intended for tracking machine learning graphs but I think there's a way to hack it to work for our image testing use case. But 1) should finish #1037 first and 2) this will need some experimenting.

@seisman
Copy link
Member Author

seisman commented Mar 18, 2021

They should be able to push the button, that DAGsHub repo is just a clone of the GitHub repo so shouldn't have any permissions issue (all public anyway).

OK. At least we can pull the images and review them.

Base automatically changed from data_version_control to master March 18, 2021 03:57
@weiji14 weiji14 added the maintenance Boring but important stuff for the core devs label Mar 18, 2021
@seisman seisman closed this Mar 22, 2021
@seisman seisman deleted the fix-test-basemap-polar branch March 22, 2021 01:35
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
maintenance Boring but important stuff for the core devs
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@seisman @weiji14