Skip to content

Migrate Figure.colorbar tests to use dvc#1143

Merged
seisman merged 24 commits intomasterfrom
test-colobar-dvc
Apr 2, 2021
Merged

Migrate Figure.colorbar tests to use dvc#1143
seisman merged 24 commits intomasterfrom
test-colobar-dvc

Conversation

@willschlitzer
Copy link
Contributor

@willschlitzer willschlitzer commented Mar 28, 2021

As discussed in #1131, this moves the reference figures in fig.colorbar to use dvc and replaces the tests using @check_figures_equal.

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

@willschlitzer willschlitzer added maintenance Boring but important stuff for the core devs skip-changelog Skip adding Pull Request to changelog labels Mar 28, 2021
@willschlitzer willschlitzer added this to the 0.4.0 milestone Mar 28, 2021
@willschlitzer willschlitzer mentioned this pull request Mar 28, 2021
Closed
28 tasks
@github-actions
Copy link
Contributor

github-actions bot commented Mar 30, 2021 •
edited
Loading

Summary of changed images

This is an auto-generated report of images that have changed on the DVC remote

Status Path
added pygmt/tests/baseline/test_colorbar_box.png
added pygmt/tests/baseline/test_colorbar_box_with_fill.png
added pygmt/tests/baseline/test_colorbar_scaled_z_values.png
added pygmt/tests/baseline/test_colorbar_shading_boolean.png
added pygmt/tests/baseline/test_colorbar_shading_list.png
added pygmt/tests/baseline/test_colorbar_truncated_to_zlow_zhigh.png

Image diff(s)

Details
  • pygmt/tests/baseline/test_colorbar_box.png

  • pygmt/tests/baseline/test_colorbar_box_with_fill.png

  • pygmt/tests/baseline/test_colorbar_scaled_z_values.png

  • pygmt/tests/baseline/test_colorbar_shading_boolean.png

  • pygmt/tests/baseline/test_colorbar_shading_list.png

  • pygmt/tests/baseline/test_colorbar_truncated_to_zlow_zhigh.png

Report last updated at commit 751b212

@willschlitzer willschlitzer changed the title WIP: Migrate fig.colorbar tests to use dvc Migrate fig.colorbar tests to use dvc Mar 30, 2021
@willschlitzer willschlitzer marked this pull request as ready for review March 30, 2021 16:38
@seisman
Copy link
Member

seisman commented Mar 30, 2021

  1. Add basemaps?

image

Do you think we should add a basemap to these tests, so that at least we know the colorbar is plotted at the correct positions.

  1. If I understand it correctly, calling Figure.colorbar() without any arguments also works. I think we should also test it. I think most people usually call Figure.colorbar() first and add more arguments to change the appearance.

image

Colorbars in two tests have messy annotations. Perhaps we should add frame=True to Figure.colorbar()?

@weiji14
Copy link
Member

weiji14 commented Mar 31, 2021 •
edited
Loading

  1. Add basemaps?

image

Do you think we should add a basemap to these tests, so that at least we know the colorbar is plotted at the correct positions.

Nah, I think it's fine as it is.

  1. If I understand it correctly, calling Figure.colorbar() without any arguments also works. I think we should also test it. I think most people usually call Figure.colorbar() first and add more arguments to change the appearance.

Yes, but is it important enough to test this? Also, adding a test seems beyond the scope of this migration PR.

image

Colorbars in two tests have messy annotations. Perhaps we should add frame=True to Figure.colorbar()?

Yes, probably good to change the annotation interval so the axis labels are visible.

seisman
seisman reviewed Apr 1, 2021
View reviewed changes
@willschlitzer @seisman
Apply suggestions from code review
09aa38b 
Co-authored-by: Dongdong Tian <seisman.info@gmail.com>
@seisman seisman changed the title Migrate fig.colorbar tests to use dvc Migrate Figure.colorbar tests to use dvc Apr 2, 2021
@seisman seisman merged commit e37d361 into master Apr 2, 2021
@seisman seisman deleted the test-colobar-dvc branch April 2, 2021 14:42
sixy6e pushed a commit to sixy6e/pygmt that referenced this pull request Dec 21, 2022
@willschlitzer @weiji14 @seisman
Migrate Figure.colorbar tests to use dvc (GenericMappingTools#1143)
b920a13 
* remove test_colorbar_using_paper_coordinates_horizontal()
* remove test_colorbar_box_with_rounded_corners()
* remove test_colorbar_shading_string()
* remove test_colorbar_positioned_using_justification_code()
* remove test_colorbar_box_with_secondary_border()
* remove test_colorbar_box_with_clearance()
* remove test_colorbar_box_with_pen()
* remove test_colorbar_box_with_offset_background()
* update test_colorbar_shading_boolean() to use mpl_image_compare
* remove test_color_shading_float and update test_colorbar_shading_list to use mpl_image_compare
* remove .png files, add .dvc files
* remove test_colorbar_positioned_using_normalized_coords()
* remove test_colorbar_positioned_using_map_coordinates()
* remove test_colorbar_using_paper_coordinates()

Co-authored-by: Wei Ji <23487320+weiji14@users.noreply.github.com>
Co-authored-by: Dongdong Tian <seisman.info@gmail.com>
@weiji14 weiji14 mentioned this pull request Jan 10, 2024
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@seisman seisman seisman approved these changes

Assignees

No one assigned

Labels

maintenance Boring but important stuff for the core devs skip-changelog Skip adding Pull Request to changelog

Projects

None yet

Milestone

0.4.0

Development

Successfully merging this pull request may close these issues.

3 participants

@willschlitzer @seisman @weiji14