From db3cbe567671fbb808c88ba2468cdbe08ad2f4a7 Mon Sep 17 00:00:00 2001 From: Dongdong Tian Date: Sat, 16 Jul 2022 16:35:22 +0800 Subject: [PATCH 01/11] Let GMTDataArrayAccessor check if the grid source file exists --- pygmt/accessors.py | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/pygmt/accessors.py b/pygmt/accessors.py index 5c67efb3354..ba542d331bf 100644 --- a/pygmt/accessors.py +++ b/pygmt/accessors.py @@ -1,6 +1,8 @@ """ GMT accessor methods. """ +from pathlib import Path + import xarray as xr from pygmt.exceptions import GMTInvalidInput from pygmt.src.grdinfo import grdinfo @@ -29,6 +31,8 @@ def __init__(self, xarray_obj): self._obj = xarray_obj try: self._source = self._obj.encoding["source"] # filepath to NetCDF source + if not Path(self._source).exists(): + raise ValueError(f"Grid source file {self._source} doesn't exist.") # Get grid registration and grid type from the last two columns of # the shortened summary information of `grdinfo`. self._registration, self._gtype = map( From bae08d19aca8ca7424b39baff1339d4d3f398660 Mon Sep 17 00:00:00 2001 From: Dongdong Tian Date: Tue, 26 Jul 2022 13:30:41 +0800 Subject: [PATCH 02/11] Raise FileNotFoundError instead Co-authored-by: Wei Ji <23487320+weiji14@users.noreply.github.com> --- pygmt/accessors.py | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/pygmt/accessors.py b/pygmt/accessors.py index ba542d331bf..d93c58ee70a 100644 --- a/pygmt/accessors.py +++ b/pygmt/accessors.py @@ -32,13 +32,15 @@ def __init__(self, xarray_obj): try: self._source = self._obj.encoding["source"] # filepath to NetCDF source if not Path(self._source).exists(): - raise ValueError(f"Grid source file {self._source} doesn't exist.") + raise FileNotFoundError( + f"Grid source file {self._source} doesn't exist." + ) # Get grid registration and grid type from the last two columns of # the shortened summary information of `grdinfo`. self._registration, self._gtype = map( int, grdinfo(self._source, per_column="n").split()[-2:] ) - except (KeyError, ValueError): + except (KeyError, ValueError, FileNotFoundError): self._registration = 0 # Default to Gridline registration self._gtype = 0 # Default to Cartesian grid type From 37869d470b45773c02f731ec8c006651d1db4c9b Mon Sep 17 00:00:00 2001 From: Dongdong Tian Date: Wed, 27 Jul 2022 00:00:07 +0800 Subject: [PATCH 03/11] Add a unit test --- pygmt/tests/test_accessor.py | 35 +++++++++++++++++++++++++++++++++++ 1 file changed, 35 insertions(+) diff --git a/pygmt/tests/test_accessor.py b/pygmt/tests/test_accessor.py index ae3c01628a8..b406e18c4a0 100644 --- a/pygmt/tests/test_accessor.py +++ b/pygmt/tests/test_accessor.py @@ -2,11 +2,14 @@ Test the behaviour of the GMTDataArrayAccessor class. """ import os +from calendar import day_abbr +from pathlib import Path import pytest import xarray as xr from packaging.version import Version from pygmt import clib, which +from pygmt.datasets import load_earth_relief from pygmt.exceptions import GMTInvalidInput with clib.Session() as _lib: @@ -98,3 +101,35 @@ def test_accessor_sliced_datacube(): assert grid.gmt.gtype == 1 # geographic coordinate type finally: os.remove(fname) + + +def test_accessor_grid_source_file_not_found(): + """ + Check that the accessor fallbacks to the default registration and gtype + when grid.encoding["source"] is given but the file is not found. + + Address issue https://github.com/GenericMappingTools/pygmt/issues/1984. + """ + grid = load_earth_relief( + resolution="01d", region=[0, 5, -5, 5], registration="pixel" + ) + # check the original grid + assert len(grid.encoding["source"]) > 0 + assert grid.gmt.registration == 1 + assert grid.gmt.gtype == 1 + + # generate a new dataset + dataset = grid.to_dataset(name="height") + # source file is given but not found + assert len(dataset.height.encoding["source"]) > 0 + assert not Path(dataset.height.encoding["source"]).exists() + # fallback to default registration and gtype + assert dataset.height.gmt.registration == 0 + assert dataset.height.gmt.gtype == 0 + + # manually set the registration and gtype + dataset.height.gmt.registration = 1 + dataset.height.gmt.gtype = 1 + # the registration and gtype should be correct now. + assert dataset.height.gmt.registration == 1 + assert dataset.height.gmt.gtype == 1 From 7b0520ad239a130b85d8bd9b140a0e47c6d738fb Mon Sep 17 00:00:00 2001 From: Dongdong Tian Date: Wed, 27 Jul 2022 17:56:08 +0800 Subject: [PATCH 04/11] Fix the test failure --- pygmt/tests/test_accessor.py | 11 +++++++---- 1 file changed, 7 insertions(+), 4 deletions(-) diff --git a/pygmt/tests/test_accessor.py b/pygmt/tests/test_accessor.py index b406e18c4a0..1072558d391 100644 --- a/pygmt/tests/test_accessor.py +++ b/pygmt/tests/test_accessor.py @@ -2,7 +2,6 @@ Test the behaviour of the GMTDataArrayAccessor class. """ import os -from calendar import day_abbr from pathlib import Path import pytest @@ -130,6 +129,10 @@ def test_accessor_grid_source_file_not_found(): # manually set the registration and gtype dataset.height.gmt.registration = 1 dataset.height.gmt.gtype = 1 - # the registration and gtype should be correct now. - assert dataset.height.gmt.registration == 1 - assert dataset.height.gmt.gtype == 1 + # the registration and gtype still have default values. + # Quote from https://docs.xarray.dev/en/stable/internals/extending-xarray.html + # > New instances, like those created from arithmetic operations or when + # > accessing a DataArray from a Dataset (ex. ds[var_name]), will have + # > new accessors created. + assert dataset.height.gmt.registration == 0 + assert dataset.height.gmt.gtype == 0 From 0e031e0c588abbc70e334cefe05cd64a1a648157 Mon Sep 17 00:00:00 2001 From: Dongdong Tian Date: Thu, 16 Feb 2023 21:30:38 +0800 Subject: [PATCH 05/11] Test with sliced grid --- pygmt/tests/test_accessor.py | 47 +++++++++++++++--------------------- 1 file changed, 20 insertions(+), 27 deletions(-) diff --git a/pygmt/tests/test_accessor.py b/pygmt/tests/test_accessor.py index 7523aab8574..2b15b3a19d9 100644 --- a/pygmt/tests/test_accessor.py +++ b/pygmt/tests/test_accessor.py @@ -108,37 +108,30 @@ def test_accessor_sliced_datacube(): os.remove(fname) -def test_accessor_grid_source_file_not_found(): +def test_accessor_grid_source_file_not_exist(): """ Check that the accessor fallbacks to the default registration and gtype - when grid.encoding["source"] is given but the file is not found. - - Address issue https://github.com/GenericMappingTools/pygmt/issues/1984. + when the grid source file (i.e., grid.encoding["source"]) doesn't exists. """ + # load the 05m earth relief grid, which is stored as tiles grid = load_earth_relief( - resolution="01d", region=[0, 5, -5, 5], registration="pixel" + resolution="05m", region=[0, 5, -5, 5], registration="pixel" ) - # check the original grid - assert len(grid.encoding["source"]) > 0 + # registration and gtype are correct assert grid.gmt.registration == 1 assert grid.gmt.gtype == 1 - - # generate a new dataset - dataset = grid.to_dataset(name="height") - # source file is given but not found - assert len(dataset.height.encoding["source"]) > 0 - assert not Path(dataset.height.encoding["source"]).exists() - # fallback to default registration and gtype - assert dataset.height.gmt.registration == 0 - assert dataset.height.gmt.gtype == 0 - - # manually set the registration and gtype - dataset.height.gmt.registration = 1 - dataset.height.gmt.gtype = 1 - # the registration and gtype still have default values. - # Quote from https://docs.xarray.dev/en/stable/internals/extending-xarray.html - # > New instances, like those created from arithmetic operations or when - # > accessing a DataArray from a Dataset (ex. ds[var_name]), will have - # > new accessors created. - assert dataset.height.gmt.registration == 0 - assert dataset.height.gmt.gtype == 0 + # the source grid file is defined but doesn't exists + assert len(grid.encoding["source"]) > 0 + assert not Path(grid.encoding["source"]).exists() + + # For a sliced grid, fallback to default registration and gtype, + # because the source grid file doesn't exists. + sliced_grid = grid[1:3, 1:3] + assert sliced_grid.gmt.registration == 0 + assert sliced_grid.gmt.gtype == 0 + + # Still possible to manually set registration and gtype + sliced_grid.gmt.registration = 1 + sliced_grid.gmt.gtype = 1 + assert sliced_grid.gmt.registration == 1 + assert sliced_grid.gmt.gtype == 1 From 9f48efc9006f5b6038934578d745bf6a136a8c9e Mon Sep 17 00:00:00 2001 From: Dongdong Tian Date: Fri, 17 Feb 2023 21:47:54 +0800 Subject: [PATCH 06/11] Fix imports order --- pygmt/tests/test_accessor.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pygmt/tests/test_accessor.py b/pygmt/tests/test_accessor.py index 75d3902c743..c97c9fda9dc 100644 --- a/pygmt/tests/test_accessor.py +++ b/pygmt/tests/test_accessor.py @@ -8,8 +8,8 @@ import pytest import xarray as xr from packaging.version import Version -from pygmt.datasets import load_earth_relief from pygmt import __gmt_version__, which +from pygmt.datasets import load_earth_relief from pygmt.exceptions import GMTInvalidInput From cbe107f7e55a44bbaa13de2641ca494762f6b190 Mon Sep 17 00:00:00 2001 From: Dongdong Tian Date: Mon, 20 Feb 2023 13:01:01 +0800 Subject: [PATCH 07/11] Apply suggestions from code review Co-authored-by: Wei Ji <23487320+weiji14@users.noreply.github.com> --- pygmt/tests/test_accessor.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/pygmt/tests/test_accessor.py b/pygmt/tests/test_accessor.py index c97c9fda9dc..b5480474209 100644 --- a/pygmt/tests/test_accessor.py +++ b/pygmt/tests/test_accessor.py @@ -118,11 +118,11 @@ def test_accessor_grid_source_file_not_exist(): assert grid.gmt.registration == 1 assert grid.gmt.gtype == 1 # the source grid file is defined but doesn't exists - assert len(grid.encoding["source"]) > 0 + assert grid.encoding["source"].endswith(".nc") assert not Path(grid.encoding["source"]).exists() # For a sliced grid, fallback to default registration and gtype, - # because the source grid file doesn't exists. + # because the source grid file doesn't exist. sliced_grid = grid[1:3, 1:3] assert sliced_grid.gmt.registration == 0 assert sliced_grid.gmt.gtype == 0 From 6bc113e2cfb786307bf909c55811bb2a3decaa3f Mon Sep 17 00:00:00 2001 From: Dongdong Tian Date: Mon, 20 Feb 2023 13:05:17 +0800 Subject: [PATCH 08/11] Refactor the codes for determining grid registration and gtype --- pygmt/accessors.py | 24 +++++++++++++----------- 1 file changed, 13 insertions(+), 11 deletions(-) diff --git a/pygmt/accessors.py b/pygmt/accessors.py index 67bb1baeecf..e73f1461ca3 100644 --- a/pygmt/accessors.py +++ b/pygmt/accessors.py @@ -52,20 +52,22 @@ class GMTDataArrayAccessor: def __init__(self, xarray_obj): self._obj = xarray_obj - try: - self._source = self._obj.encoding["source"] # filepath to NetCDF source - if not Path(self._source).exists(): - raise FileNotFoundError( - f"Grid source file {self._source} doesn't exist." + + self._source = self._obj.encoding.get("source") + if self._source is not None and Path(self._source).exists(): + try: + # Get grid registration and grid type from the last two columns of + # the shortened summary information of `grdinfo`. + self._registration, self._gtype = map( + int, grdinfo(self._source, per_column="n").split()[-2:] ) - # Get grid registration and grid type from the last two columns of - # the shortened summary information of `grdinfo`. - self._registration, self._gtype = map( - int, grdinfo(self._source, per_column="n").split()[-2:] - ) - except (KeyError, ValueError, FileNotFoundError): + except ValueError: + self._registration = 0 # Default to Gridline registration + self._gtype = 0 # Default to Cartesian grid type + else: self._registration = 0 # Default to Gridline registration self._gtype = 0 # Default to Cartesian grid type + del self._source @property def registration(self): From 4389aa1d5db107c200aa0bf4c9198910ac90d428 Mon Sep 17 00:00:00 2001 From: Dongdong Tian Date: Mon, 20 Feb 2023 13:08:29 +0800 Subject: [PATCH 09/11] Apply suggestions from code review Co-authored-by: Wei Ji <23487320+weiji14@users.noreply.github.com> --- pygmt/tests/test_accessor.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/pygmt/tests/test_accessor.py b/pygmt/tests/test_accessor.py index b5480474209..9799501f252 100644 --- a/pygmt/tests/test_accessor.py +++ b/pygmt/tests/test_accessor.py @@ -108,7 +108,7 @@ def test_accessor_sliced_datacube(): def test_accessor_grid_source_file_not_exist(): """ Check that the accessor fallbacks to the default registration and gtype - when the grid source file (i.e., grid.encoding["source"]) doesn't exists. + when the grid source file (i.e., grid.encoding["source"]) doesn't exist. """ # load the 05m earth relief grid, which is stored as tiles grid = load_earth_relief( @@ -117,7 +117,7 @@ def test_accessor_grid_source_file_not_exist(): # registration and gtype are correct assert grid.gmt.registration == 1 assert grid.gmt.gtype == 1 - # the source grid file is defined but doesn't exists + # the source grid file is defined but doesn't exist assert grid.encoding["source"].endswith(".nc") assert not Path(grid.encoding["source"]).exists() From ef79c20c16b0f4a5582a7a0b46e7d1955f237930 Mon Sep 17 00:00:00 2001 From: Dongdong Tian Date: Mon, 20 Feb 2023 13:14:40 +0800 Subject: [PATCH 10/11] Fix formatting --- pygmt/accessors.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/pygmt/accessors.py b/pygmt/accessors.py index e73f1461ca3..30195736d83 100644 --- a/pygmt/accessors.py +++ b/pygmt/accessors.py @@ -56,8 +56,8 @@ def __init__(self, xarray_obj): self._source = self._obj.encoding.get("source") if self._source is not None and Path(self._source).exists(): try: - # Get grid registration and grid type from the last two columns of - # the shortened summary information of `grdinfo`. + # Get grid registration and grid type from the last two columns + # of the shortened summary information of `grdinfo`. self._registration, self._gtype = map( int, grdinfo(self._source, per_column="n").split()[-2:] ) From 006350a78359b2d5d9197da37507247cee68eae6 Mon Sep 17 00:00:00 2001 From: Dongdong Tian Date: Mon, 20 Feb 2023 17:09:19 +0800 Subject: [PATCH 11/11] Apply suggestions from code review Co-authored-by: Michael Grund <23025878+michaelgrund@users.noreply.github.com> --- pygmt/tests/test_accessor.py | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/pygmt/tests/test_accessor.py b/pygmt/tests/test_accessor.py index 9799501f252..d9bb684d69b 100644 --- a/pygmt/tests/test_accessor.py +++ b/pygmt/tests/test_accessor.py @@ -110,14 +110,14 @@ def test_accessor_grid_source_file_not_exist(): Check that the accessor fallbacks to the default registration and gtype when the grid source file (i.e., grid.encoding["source"]) doesn't exist. """ - # load the 05m earth relief grid, which is stored as tiles + # Load the 05m earth relief grid, which is stored as tiles grid = load_earth_relief( resolution="05m", region=[0, 5, -5, 5], registration="pixel" ) - # registration and gtype are correct + # Registration and gtype are correct assert grid.gmt.registration == 1 assert grid.gmt.gtype == 1 - # the source grid file is defined but doesn't exist + # The source grid file is defined but doesn't exist assert grid.encoding["source"].endswith(".nc") assert not Path(grid.encoding["source"]).exists()