Skip to content
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

Update baseline images for basemap #1221

Merged
merged 4 commits into from
Apr 21, 2021
Merged

Update baseline images for basemap #1221

merged 4 commits into from
Apr 21, 2021

Conversation

seisman
Copy link
Member

@seisman seisman commented Apr 21, 2021

Description of proposed changes

Address #1217.

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

@github-actions
Copy link
Contributor

github-actions bot commented Apr 21, 2021

Summary of changed images

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

Status Path
modified pygmt/tests/baseline/test_basemap.png
modified pygmt/tests/baseline/test_basemap_compass.png
modified pygmt/tests/baseline/test_basemap_loglog.png
modified pygmt/tests/baseline/test_basemap_map_scale.png
modified pygmt/tests/baseline/test_basemap_power_axis.png
modified pygmt/tests/baseline/test_basemap_rose.png
modified pygmt/tests/baseline/test_basemap_winkel_tripel.png

Image diff(s)

Added images

Modified images

Path Old New
test_basemap.png
test_basemap_compass.png
test_basemap_loglog.png
test_basemap_map_scale.png
test_basemap_power_axis.png
test_basemap_rose.png
test_basemap_winkel_tripel.png

Report last updated at commit 7a5731c

@seisman seisman marked this pull request as draft April 21, 2021 19:33
@seisman seisman marked this pull request as ready for review April 21, 2021 19:50
@seisman seisman added this to the 0.4.0 milestone Apr 21, 2021
@seisman seisman added maintenance Boring but important stuff for the core devs skip-changelog Skip adding Pull Request to changelog labels Apr 21, 2021
@seisman
Copy link
Member Author

seisman commented Apr 21, 2021

All the new images make sense to me, except the test_basemap_polar.png one. I think it's a bug in GMT v6.2.0rc1, agree?

@maxrjones
Copy link
Member

Yes, I agree that it shows this bug: GenericMappingTools/gmt#5120. I do not think it should be allowed to pass with the new image.

@seisman
Copy link
Member Author

seisman commented Apr 21, 2021

Yes, I agree that it shows this bug: GenericMappingTools/gmt#5120.

I don't think they're exactly the same bug. The equivalent GMT CLI is:

gmt basemap -R0/360/0/1000 -JP8c -Bafg -pdf map

It should NOT append any suffix (e.g., E) since it's a Cartesian polar projection.

image

I do not think it should be allowed to pass with the new image.

Yes, I agree. I think I should keep the old test_basemap_polar.png image, and mark the test "xfail" for GMT 6.2.0rc1, right?

@maxrjones
Copy link
Member

Yes, I agree that it shows this bug: GenericMappingTools/gmt#5120.

I don't think they're exactly the same bug. The equivalent GMT CLI is:

gmt basemap -R0/360/0/1000 -JP8c -Bafg -pdf map

It should NOT append any suffix (e.g., E) since it's a Cartesian polar projection.

I agree that no units should be appended. My understanding is that right now GMT assumes that all angles given to -JP are longitude, which is a problem for both #5120 and the PyGMT test. Up to you if you want to post this in the same issue or a new one.

I do not think it should be allowed to pass with the new image.

Yes, I agree. I think I should keep the old test_basemap_polar.png image, and mark the test "xfail" for GMT 6.2.0rc1, right?

Yes to me.

@seisman
Copy link
Member Author

seisman commented Apr 21, 2021

I agree that no units should be appended. My understanding is that right now GMT assumes that all angles given to -JP are longitude, which is a problem for both #5120 and the PyGMT test. Up to you if you want to post this in the same issue or a new one.

I think it's a new one. GMT v6.1.1 does not make this assumption. I'll post a new bug report.

@seisman seisman mentioned this pull request Apr 21, 2021
3 tasks
@seisman seisman merged commit 35883df into master Apr 21, 2021
@seisman seisman deleted the gmt-6.2.0rc1/basemap branch April 21, 2021 22:25
sixy6e pushed a commit to sixy6e/pygmt that referenced this pull request Dec 21, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
maintenance Boring but important stuff for the core devs skip-changelog Skip adding Pull Request to changelog
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants