-
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
Bump to GMT 6.2.0rc1 #1217
Comments
Bumping to GMT 6.2.0rc1 will also affect several existing PRs. I think we should go ahead with bumping the version as soon as possible though (so that we can test and fix any new bugs). Is there any PR we would like to merge/finish before bumping PyGMT to use GMT 6.2.0rc1? Otherwise it will get lost under a torrent of PRs relating to fixing broken unit tests 😆 I was actually thinking of updating https://github.com/GenericMappingTools/pygmt/blob/e4cb775346814dce46555a27c0c94f8e0aed6f04/.github/workflows/dvc-diff.yml to have a side-by-side before/after comparison of the images (instead of just having a single new image shown). Unsure if I have time this week to update that workflow (have a presentation and my PhD defence to prep for) but will see. |
I can work on this. |
I know we haven't updated the documentation for the maintainer review process yet, but I think these are good examples of when to use the 'request a review' option for pinging pygmt-maintainers. At least for me, I usually periodically skim the PRs for any that have been marked 'ready for review' or 'final review call' but will notice specific review requests immediately. |
I've merged #1218, so now the CI tests are running with GMT 6.2.0rc1, and ~100 tests fail, due to the modern theme changes introduced in GMT 6.2. What we need to do to make the tests pass again:
|
I'll handle all the test_grd*.py tests in a single PR if that's ok. |
OK to me. |
Cool, can we also agree on a standardized PR title? E.g. "Update fig.??? baseline images for GMT 6.2.0rc1"? |
Good. |
I think we may be encountering something flaky in PyGMT's test suite. There's 3 separate cases (opened by 3 different people) where using
This reminds me of https://github.com/GenericMappingTools/pygmt/pull/476/files#r453429872 (which motivated the use of a |
For makecpt, edit: I have not yet been able to figure out a solution. The two makecpt tests fail if there is a docstring example that imports pygmt and instantiates a figure (e.g., |
These flaky tests are difficult to debug. Perhaps we can use one of the "flaky plugins" to rerun the failing tests as a workaround. I think they should pass, no? |
Just tried pytest-rerunfailures. Not lucky. It re-runs a test immediately after it fails, rather than re-running all failing tests after all tests finish. |
Yeah, from experience, I found the listed pytest flaky plugins to be a bit of a hit and miss in PyGMT's case. Also tried pytest-reverse and I'm getting lots of failures on |
I understand the original logic behind a single GMT session for all tests in #327 (comment). Still, I don't expect that users will be attempting to use the entire PyGMT library in a single session, which is the goal of the test suite. So I think it would be worth revisiting this decision. Could it be possible to periodically test the examples/tutorials against baseline images to ensure that producing multiple plots in a single session is consistent and have the unit tests each use individual sessions? |
Ok, I've opened up a separate issue to decide on whether PyGMT should have single/separate GMT session(s) at #1242. Let's continue the test flakiness discussion there. For the purposes of quickly resolving this GMT 6.2.0rc1 baseline image update issue, specifically for |
Since option 2 will reverse the problem (i.e., fail with |
Option 1 👍 |
A little progress on the flaky tests. Similar to
If I delete the file
I didn't check all the tests, but it seems some baseline images are incorrect, but the tests on master branch pass. For example, for the test
As far as I understand it, test files like |
Actually, on the master branch, the |
Here is a minimal example to reproduce the flaky tests we're having:
Just in case you don't know it:
The output of the two "gmt get" calls is:
The resulting images are:
Apparently, these two images diff due to the differences of |
Thanks for the the minimal example @seisman. I will see what I can find about this. |
No solution yet but it seems to be an issue with |
GenericMappingTools/gmt#5178 fixes the minimal example and make these tests fail (some now in a proper way):
But, the two |
Thanks @seisman and @meghanrjones for tracking this down and fixing the bug. Sounds like we might need a GMT 6.2.0rc2? Let's put an xfail for the |
I agree 6.2.0rc2 will be needed soon. Even though the gmt-dev CI tests are passing, I am still concerned about my local fatal error in
I plan to look into this more tomorrow, but in the meantime this is the specific error that I get when trying to run |
Agree on finding out what is causing the Edit: I can't reproduce the fatal error on Linux, and the 'macOS-11.0 - GMT master' test didn't seem to crash either at https://github.com/GenericMappingTools/pygmt/runs/2463360992?check_suite_focus=true#step:15:272. |
Thanks for checking. It was a silly mistake on my part in continuing on in debug mode even after being done with debugging so I ran into an assert statement 🤦♀️ |
The first release candidate of GMT 6.2.0 has been released (https://github.com/GenericMappingTools/gmt/releases/tag/6.2.0rc1) today.
Now GMT 6.2.0rc1 is available in the devel channel on conda-forge (https://anaconda.org/conda-forge/gmt/) and can be installed using:
We need to:
The text was updated successfully, but these errors were encountered: