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

Bump the minimum required GMT version to 6.3.0 #1649

Merged
merged 9 commits into from
Dec 13, 2021
Merged

Conversation

maxrjones
Copy link
Member

Description of proposed changes

Bump the minimum required GMT version from 6.2.0 to 6.3.0. Also update GitHub Actions CI workflows to use GMT 6.3.0.

Relates to #1644. Supersedes #1321.

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.

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 added the maintenance Boring but important stuff for the core devs label Dec 8, 2021
@seisman seisman added this to the 0.6.0 milestone Dec 8, 2021
@seisman
Copy link
Member

seisman commented Dec 8, 2021

The gallery example examples/gallery/embellishments/colorbars_multiple.py fails with GMT 6.3.0. Need to see if it's an upstream bug.

@weiji14
Copy link
Member

weiji14 commented Dec 9, 2021

The gallery example examples/gallery/embellishments/colorbars_multiple.py fails with GMT 6.3.0. Need to see if it's an upstream bug.

Traceback

Unexpected failing examples:
/home/runner/work/pygmt/pygmt/examples/gallery/embellishments/colorbars_multiple.py failed leaving traceback:
Traceback (most recent call last):
  File "/home/runner/work/pygmt/pygmt/examples/gallery/embellishments/colorbars_multiple.py", line 31, in <module>
    fig.grdimage(grid=grid_globe, projection="R?", region="g", frame=True)
  File "/usr/share/miniconda3/envs/pygmt/lib/python3.9/site-packages/pygmt/helpers/decorators.py", line 586, in new_module
    return module_func(*args, **kwargs)
  File "/usr/share/miniconda3/envs/pygmt/lib/python3.9/site-packages/pygmt/helpers/decorators.py", line 726, in new_module
    return module_func(*args, **kwargs)
  File "/usr/share/miniconda3/envs/pygmt/lib/python3.9/site-packages/pygmt/src/grdimage.py", line 175, in grdimage
    lib.call_module("grdimage", arg_str)
  File "/usr/share/miniconda3/envs/pygmt/lib/python3.9/site-packages/pygmt/clib/session.py", line 500, in call_module
    raise GMTCLibError(
pygmt.exceptions.GMTCLibError: Module 'grdimage' failed with status code 72:
Error: e [ERROR]: Option -B parsing failure. Correct syntax:
Error: e [ERROR]: Offending option -B

Relevant line that is failing

fig.grdimage(grid=grid_globe, projection="R?", region="g", frame=True)

I would prefer that the Docs CI tests passes before this PR is merged. Maybe find a way to modify that gallery example a bit to workaround the upstream bug.

@seisman
Copy link
Member

seisman commented Dec 9, 2021

Maybe find a way to modify that gallery example a bit to workaround the upstream bug.

Removing frame=True from the two grdimage calls will make the example work, but I'm not sure if it's an upstream bug or fix.

@weiji14
Copy link
Member

weiji14 commented Dec 9, 2021

Maybe find a way to modify that gallery example a bit to workaround the upstream bug.

Removing frame=True from the two grdimage calls will make the example work, but I'm not sure if it's an upstream bug or fix.

Yeah, not sure whether it's a problem with grdimage or subplot. I don't see frame/-B declared in the subplot call either which is strange. Is there a default hidden frame/-B setting for subplots now? I couldn't find anything obvious about changes to grdimage or subplot in the changelog at https://docs.generic-mapping-tools.org/6.3/changes.html, so not sure where the bug lies, or if it's just a user error 😅

Either way, we probably do need to remove frame=True since I don't think there's plans for a GMT 6.3.1

@maxrjones
Copy link
Member Author

maxrjones commented Dec 9, 2021

Maybe find a way to modify that gallery example a bit to workaround the upstream bug.

Removing frame=True from the two grdimage calls will make the example work, but I'm not sure if it's an upstream bug or fix.

In GMT 6.3, it's not longer working to use -B without any arguments inside subplot mode:

# Works in 6.2. and 6.3
gmt grdimage @earth_relief_05m -R0/10/0/10 -B -png test
# Works in 6.2 but not 6.3
gmt begin test2 png
    gmt subplot begin 1x2 -Fs15c/8c -A -M0.5c
    gmt subplot set 0
        gmt grdimage @earth_relief_05m -R0/10/0/10 -B
    gmt subplot set 1
        gmt grdimage @earth_relief_05m -R0/10/0/10 -B
    gmt subplot end
gmt end show

I can try to track down this regression.

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.

Ok ✅ from me. The new dcw-gmt package's GMT 6.3.0 requirement is breaking the main branch's CI, so best to move ahead with this (even though it is a busy AGU week)!

@maxrjones
Copy link
Member Author

Ok ✅ from me. The new dcw-gmt package's GMT 6.3.0 requirement is breaking the main branch's CI, so best to move ahead with this (even though it is a busy AGU week)!

Yes, sorry for the awful timing. Hopefully we'll have a patch release for the dcw bug soon (GenericMappingTools/dcw-gmt#39), but merging this will be quicker.

@maxrjones maxrjones mentioned this pull request Dec 13, 2021
31 tasks
@maxrjones maxrjones merged commit ffe338e into main Dec 13, 2021
@maxrjones maxrjones deleted the bump-gmt-6.3.0 branch December 13, 2021 19:31
sixy6e pushed a commit to sixy6e/pygmt that referenced this pull request Dec 21, 2022
)

* Update install.rst

* Update cache_data.yaml

* Update ci_docs.yml

* Update ci_tests.yaml

* Update environment.yml

* Update session.py

* Update test_clib.py

* Update README.rst
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
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants