Skip to content

Conversation

weiji14
Copy link
Member

@weiji14 weiji14 commented Sep 15, 2025

Description of proposed changes

The test_binstats_quantile test (introduced in #3012) is failing on the legacy CI tests. Trying to see if it's due to xarray/pandas/numpy or using old version of GMT 6.4 Edit: seems to be that GMT 6.4 gives different results to GMT 6.5+, most likely due to changes in GenericMappingTools/gmt#8243

Traceback from https://github.com/GenericMappingTools/pygmt/actions/runs/16895808645/job/47865247702#step:6:1098

___________________________ test_binstats_quantile ____________________________
    def test_binstats_quantile():
        """
        Test binstats quantile statistic functionality.
        """
        temp_grid = binstats(
            data="@capitals.gmt",
            spacing=5,
            statistic="quantile",
            quantile_value=75,
            search_radius="1000k",
            aspatial="2=population",
            region="g",
        )
        assert temp_grid.dims == ("y", "x")
        assert temp_grid.gmt.gtype is GridType.CARTESIAN
        assert temp_grid.gmt.registration is GridRegistration.GRIDLINE
        npt.assert_allclose(temp_grid.max(), 15047685)
>       npt.assert_allclose(temp_grid.min(), 53)
..\pygmt\tests\test_binstats.py:71: 
_ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _
args = (<function assert_allclose.<locals>.compare at 0x000001D761A02E80>, array(0., dtype=float32), array(53))
kwds = {'equal_nan': True, 'err_msg': '', 'header': 'Not equal to tolerance rtol=1e-07, atol=0', 'verbose': True}
    @wraps(func)
    def inner(*args, **kwds):
        with self._recreate_cm():
>           return func(*args, **kwds)
E           AssertionError: 
E           Not equal to tolerance rtol=1e-07, atol=0
E           
E           Mismatched elements: 1 / 1 (100%)
E           Max absolute difference: 53.
E           Max relative difference: 1.
E            x: array(0., dtype=float32)
E            y: array(53)
C:\hostedtoolcache\windows\Python\3.11.9\x64\Lib\contextlib.py:81: AssertionError

Output of pygmt.show_versions() from CI:

PyGMT information:
  version: v0.17.0.dev39+g354b50a0f
System information:
  python: 3.11.9 | packaged by conda-forge | (main, Apr 19 2024, 18:36:13) [GCC 12.3.0]
  executable: /home/runner/micromamba/envs/pygmt/bin/python
  machine: Linux-6.8.0-1031-azure-x86_64-with-glibc2.35
Dependency information:
  numpy: 1.26.4
  pandas: 2.3.1
  xarray: 2025.7.1
  packaging: 24.2
  contextily: 1.5.2
  geopandas: 1.0.1
  IPython: 9.4.0
  pyarrow: 16.1.0
  rioxarray: 0.19.0
  gdal: 3.8.5
  ghostscript: 9.56.1
GMT library information:
  version: 6.4.0
  padding: 2
  share dir: /home/runner/micromamba/envs/pygmt/share/gmt
  plugin dir: /home/runner/micromamba/envs/pygmt/lib/gmt/plugins
  library path: /home/runner/micromamba/envs/pygmt/lib/libgmt.so
  cores: 4
  grid layout: rows
  image layout: 
  binary version: 6.4.0

Patches #3012

Preview:

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 command is:

  • /format: automatically format and lint the code

@weiji14 weiji14 self-assigned this Sep 15, 2025
@weiji14 weiji14 added the maintenance Boring but important stuff for the core devs label Sep 15, 2025
@weiji14 weiji14 changed the title WIP: Try using skipna=True in min() for test_binstats_quantile Fix test_binstats_quantile on GMT 6.4 using different min/median/mean values Sep 15, 2025
Copy link
Member Author

@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.

This PR probably won't be needed if #4017 goes ahead, depends on whether we can get #4018 to work before the next release.

Comment on lines 74 to 81
if Version(__gmt_version__) > Version("6.4.0"):
npt.assert_allclose(temp_grid.min(), 53)
npt.assert_allclose(temp_grid.median(), 543664.5)
npt.assert_allclose(temp_grid.mean(), 1661363.6)
else: # TODO(GMT>=6.5.0): Remove if-condition with different min/median/mean values
npt.assert_allclose(temp_grid.min(), 0)
npt.assert_allclose(temp_grid.median(), 330700.0)
npt.assert_allclose(temp_grid.mean(), 1459889.1)
Copy link
Member Author

Choose a reason for hiding this comment

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

Unsure which values are the correct ones. Do we want to check in upstream GMT on what might have changed binstat from GMT 6.4 to 6.5?

Copy link
Member

Choose a reason for hiding this comment

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

Likely related to GenericMappingTools/gmt#8243.

Copy link
Member Author

Choose a reason for hiding this comment

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

Thanks for the reminder, forgot about that thread! Have changed to an xfail in commit 1dc2e42

@weiji14 weiji14 changed the title Fix test_binstats_quantile on GMT 6.4 using different min/median/mean values Mark test_binstats_quantile as xfail in the GMT Legacy Tests workflow Sep 16, 2025
@weiji14 weiji14 added the skip-changelog Skip adding Pull Request to changelog label Sep 16, 2025
@weiji14 weiji14 marked this pull request as ready for review September 16, 2025 03:49
@seisman seisman added this to the 0.17.0 milestone Sep 16, 2025
@weiji14 weiji14 merged commit a9e3881 into main Sep 16, 2025
21 of 24 checks passed
@weiji14 weiji14 deleted the fix_test_binstats_quantile branch September 16, 2025 05:32
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