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

load_earth_relief now can load gebco, gebcosi and synbath datasets with GMT 6.3 #2281

Merged
merged 5 commits into from
Dec 29, 2022

Conversation

seisman
Copy link
Member

@seisman seisman commented Dec 28, 2022

Description of proposed changes

As mentioned in #2225 (comment), after PR #2266 is merged, gebco, gebcosi and synbath dataset should work with GMT 6.3. This PR remove the exception when gebco/gebcosi/synbath datasets are used with GMT 6.3.

Fixes #2225.

Notes that:

Some tests fail with GMT 6.3 in the GMT Legacy Tests workflow (https://github.com/GenericMappingTools/pygmt/actions/runs/3784213311). These tests should pass after the changes in this PR.

Let's see the workflow results trigged by this PR https://github.com/GenericMappingTools/pygmt/actions/runs/3795048765.

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 wrapping a new module, open a 'Wrap new GMT module' issue and submit reasonably-sized PRs.
  • If adding new functionality, add an example to docstrings or tutorials.
  • Use underscores (not hyphens) in names of Python files and directories.

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

@seisman seisman marked this pull request as draft December 28, 2022 15:43
@seisman seisman marked this pull request as ready for review December 28, 2022 15:43
@seisman seisman added the enhancement Improving an existing feature label Dec 28, 2022
@seisman seisman added this to the 0.8.0 milestone Dec 28, 2022
@yvonnefroehlich yvonnefroehlich mentioned this pull request Dec 28, 2022
65 tasks
@seisman
Copy link
Member Author

seisman commented Dec 28, 2022

Let's see the workflow results trigged by this PR GenericMappingTools/pygmt/actions/runs/3795048765.

It seems there are some failures in the GMT Legacy Tests, some are caused by the missing IPython package and some are doctests.

But at least the tests with gebco/gebcosi/synbath now pass.

@seisman
Copy link
Member Author

seisman commented Dec 28, 2022

Let's see the workflow results trigged by this PR GenericMappingTools/pygmt/actions/runs/3795048765.

It seems there are some failures in the GMT Legacy Tests, some are caused by the missing IPython package and some are doctests.

But at least the tests with gebco/gebcosi/synbath now pass.

Some TODOs:

  1. Update the release checklist to check the "GMT Legacy Tests" to make sure the new release still work with old GMT versions
  2. Update the release checklist to check the "GMT Dev Tests" to make sure the new release will work with GMT dev version.
  3. IPython is not installed in the "GMT Legacy Tests"
  4. It's likely that the "Tests" workflow doesn't run the full tests (including all doctests) on Wednesday
    run: make fulltest PYTEST_EXTRA="-r P"

@weiji14
Copy link
Member

weiji14 commented Dec 28, 2022

  1. Update the release checklist to check the "GMT Legacy Tests" to make sure the new release still work with old GMT versions
  2. Update the release checklist to check the "GMT Dev Tests" to make sure the new release will work with GMT dev version.

Maybe do this after the PyGMT v0.8.0 release? We'll just manually verify this for now, so that the release isn't delayed any longer.

  1. IPython is not installed in the "GMT Legacy Tests"
  2. It's likely that the "Tests" workflow doesn't run the full tests (including all doctests) on Wednesday

Opened PR at #2283 to add ipython. Should the "GMT Legacy Tests" run the full tests (i.e. make fulltest) or just the regular tests without docstrings (i.e. make test)? Edit: Sorry, forgot that the legacy tests uses make test_no_images in #2079.

Copy link
Member

@weiji14 weiji14 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Changes should be ok. Maybe it's easier to merge this PR first, and then we can run the GMT Legacy Tests properly in #2283.

@weiji14 weiji14 added the final review call This PR requires final review and approval from a second reviewer label Dec 29, 2022
@seisman seisman merged commit 21ef836 into main Dec 29, 2022
@seisman seisman deleted the gmt-6.3-gebco-synbath branch December 29, 2022 04:22
@seisman seisman removed the final review call This PR requires final review and approval from a second reviewer label Dec 29, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement Improving an existing feature
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Improve the load_earth_relief and load_earth_age functions
2 participants