-
Notifications
You must be signed in to change notification settings - Fork 224
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’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
CI: Fix and simplify the dvc-diff workflow #2549
Conversation
Summary of changed imagesThis is an auto-generated report of images that have changed on the DVC remote
Image diff(s)Added images
Modified images
Report last updated at commit bf5c6f6 |
Summary of changed imagesThis is an auto-generated report of images that have changed on the DVC remote
Image diff(s)Added images
Modified images
Report last updated at commit befa95b |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Just a small suggestion, but otherwise looks good!
pygmt/tests/test_basemap.py
Outdated
fig.basemap(region=[10, 70, -3, 8], projection="X10c/6c", frame="afg") | ||
return fig | ||
|
||
|
||
@pytest.mark.mpl_image_compare | ||
def test_basemap_loglog(): | ||
def test_basemap_added(): | ||
""" | ||
Create a loglog basemap plot. | ||
Create a simple basemap plot. | ||
""" | ||
fig = Figure() | ||
fig.basemap( | ||
region=[1, 10000, 1e20, 1e25], | ||
projection="X16cl/12cl", | ||
frame=["WS", "x2+lWavelength", "ya1pf3+lPower"], | ||
) | ||
fig.basemap(region=[10, 70, -20, 20], projection="X10c/6c", frame="afg") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Revert changes in b7ad43b before merging
Remember to do the revert!
body: ${{ steps.image-diff.outputs.report }} | ||
edit-mode: replace | ||
# create/update PR comment | ||
cml comment update report.md |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nice that this simplifies so many lines of code!
# Produce the markdown diff report, which should look like: | ||
# ## Summary of changed images | ||
# |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Could update the preview a little bit below here
# | Status | Path |
# |----------|-------------------------------------|
# | added | pygmt/tests/baseline/test_image.png |
# | deleted | pygmt/tests/baseline/test_image2.png |
# | modified | pygmt/tests/baseline/test_image3.png |
+
+ ## Image diff(s)
+
+ <details>
+ ...
+ </details>
+
+ Report last updated at commit abcdef
Description of proposed changes
Fix the dvc-diff workflow by setting up nodejs to v16, following the workaround in iterative/cml#1377.
The dvc-diff report was posted using
peter-evans/find-comment
andpeter-evans/create-or-update-comment
in the old workflow. These steps are removed and the diff report is posted using a single commandcml comment update report.md
.Comment #2549 (comment) is the comment posted by the old workflow and #2549 (comment) is the comment posted by the new workflow (i.e.,
cml comment update report.md
).TODO: