From 2f3819d9def6d0240b83ac789edf1a3f6c233ebd Mon Sep 17 00:00:00 2001 From: Panu Lahtinen Date: Wed, 19 Apr 2023 15:32:17 +0300 Subject: [PATCH 1/5] Add a failing test for converting 1D array to CF --- satpy/tests/writer_tests/test_cf.py | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/satpy/tests/writer_tests/test_cf.py b/satpy/tests/writer_tests/test_cf.py index b89c6a9813..8e6a33f9ab 100644 --- a/satpy/tests/writer_tests/test_cf.py +++ b/satpy/tests/writer_tests/test_cf.py @@ -605,6 +605,16 @@ def test_da2cf(self): attrs_expected_flat.pop('int') self.assertDictWithArraysEqual(res_flat.attrs, attrs_expected_flat) + def test_da2cf_one_dimensional_array(self): + """Test the conversion of an 1d DataArray to a CF-compatible DataArray.""" + import xarray as xr + + from satpy.writers.cf_writer import CFWriter + + arr = xr.DataArray(np.array([1, 2, 3, 4]), attrs={}, dims=('y',), + coords={'y': [0, 1, 2, 3], 'acq_time': ('y', [0, 1, 2, 3])}) + _ = CFWriter.da2cf(arr) + @mock.patch('satpy.writers.cf_writer.CFWriter.__init__', return_value=None) def test_collect_datasets(self, *mocks): """Test collecting CF datasets from a DataArray objects.""" From 8a829bf41df6a185ae432323b0ef7d2ed6f2b80e Mon Sep 17 00:00:00 2001 From: Panu Lahtinen Date: Wed, 19 Apr 2023 15:33:14 +0300 Subject: [PATCH 2/5] Fix assumption that data has both x and y coordinates --- satpy/writers/cf_writer.py | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/satpy/writers/cf_writer.py b/satpy/writers/cf_writer.py index 6a0d265a08..164d070c12 100644 --- a/satpy/writers/cf_writer.py +++ b/satpy/writers/cf_writer.py @@ -745,8 +745,12 @@ def _try_to_get_crs(new_data): @staticmethod def _try_get_units_from_coords(new_data): for c in "xy": - if "units" in new_data.coords[c].attrs: - return new_data.coords[c].attrs["units"] + try: + if "units" in new_data.coords[c].attrs: + return new_data.coords[c].attrs["units"] + except KeyError: + # If the data has only 1 dimension, it has only one of x or y coords + pass @staticmethod def _encode_xy_coords_projected(new_data): From 6d0073eec1cc8291948122261a2c92f9769e94d9 Mon Sep 17 00:00:00 2001 From: Panu Lahtinen Date: Wed, 19 Apr 2023 15:55:09 +0300 Subject: [PATCH 3/5] Refactor to try reduce cyclomatic complexity --- satpy/writers/cf_writer.py | 17 +++++++++++------ 1 file changed, 11 insertions(+), 6 deletions(-) diff --git a/satpy/writers/cf_writer.py b/satpy/writers/cf_writer.py index 164d070c12..0b4af4f122 100644 --- a/satpy/writers/cf_writer.py +++ b/satpy/writers/cf_writer.py @@ -745,12 +745,17 @@ def _try_to_get_crs(new_data): @staticmethod def _try_get_units_from_coords(new_data): for c in "xy": - try: - if "units" in new_data.coords[c].attrs: - return new_data.coords[c].attrs["units"] - except KeyError: - # If the data has only 1 dimension, it has only one of x or y coords - pass + attrs = CFWriter._get_coord_attrs(new_data, c) + if "units" in attrs: + return attrs["units"] + + @staticmethod + def _get_coord_attrs(new_data, coord): + try: + return new_data.coords[coord].attrs + except KeyError: + # If the data has only 1 dimension, it has only one of x or y coords + return {} @staticmethod def _encode_xy_coords_projected(new_data): From 73df6cf85aade0e421a869ad1f48f95d3899e39f Mon Sep 17 00:00:00 2001 From: Panu Lahtinen Date: Wed, 19 Apr 2023 16:03:40 +0300 Subject: [PATCH 4/5] Use contextlib.suppress() to simplify structure --- satpy/writers/cf_writer.py | 16 +++++----------- 1 file changed, 5 insertions(+), 11 deletions(-) diff --git a/satpy/writers/cf_writer.py b/satpy/writers/cf_writer.py index 0b4af4f122..0cfb808425 100644 --- a/satpy/writers/cf_writer.py +++ b/satpy/writers/cf_writer.py @@ -160,6 +160,7 @@ import logging import warnings from collections import OrderedDict, defaultdict +from contextlib import suppress from datetime import datetime import numpy as np @@ -745,17 +746,10 @@ def _try_to_get_crs(new_data): @staticmethod def _try_get_units_from_coords(new_data): for c in "xy": - attrs = CFWriter._get_coord_attrs(new_data, c) - if "units" in attrs: - return attrs["units"] - - @staticmethod - def _get_coord_attrs(new_data, coord): - try: - return new_data.coords[coord].attrs - except KeyError: - # If the data has only 1 dimension, it has only one of x or y coords - return {} + with suppress(KeyError): + # If the data has only 1 dimension, it has only one of x or y coords + if "units" in new_data.coords[c].attrs: + return new_data.coords[c].attrs["units"] @staticmethod def _encode_xy_coords_projected(new_data): From 6dae14e2deefc95fecc9a4576ded7ae8b98351b0 Mon Sep 17 00:00:00 2001 From: Panu Lahtinen Date: Thu, 20 Apr 2023 09:34:20 +0300 Subject: [PATCH 5/5] Move common un-mocked imports out of methods/functions --- satpy/tests/writer_tests/test_cf.py | 97 +++-------------------------- 1 file changed, 7 insertions(+), 90 deletions(-) diff --git a/satpy/tests/writer_tests/test_cf.py b/satpy/tests/writer_tests/test_cf.py index 687645e896..100d6b75f2 100644 --- a/satpy/tests/writer_tests/test_cf.py +++ b/satpy/tests/writer_tests/test_cf.py @@ -17,6 +17,7 @@ # satpy. If not, see . """Tests for the CF writer.""" +import json import logging import os import tempfile @@ -26,10 +27,13 @@ from datetime import datetime from unittest import mock +import dask.array as da import numpy as np +import pyresample.geometry import pytest import xarray as xr from packaging.version import Version +from pyresample import create_area_def from satpy import Scene from satpy.tests.utils import make_dsq @@ -73,13 +77,11 @@ def test_init(self): """Test initializing the CFWriter class.""" from satpy.writers import configs_for_writer from satpy.writers.cf_writer import CFWriter + CFWriter(config_files=list(configs_for_writer('cf'))[0]) def test_save_array(self): """Test saving an array to netcdf/cf.""" - import xarray as xr - - from satpy import Scene scn = Scene() start_time = datetime(2018, 5, 30, 10, 0) end_time = datetime(2018, 5, 30, 10, 15) @@ -97,10 +99,6 @@ def test_save_array(self): def test_save_array_coords(self): """Test saving array with coordinates.""" - import numpy as np - import xarray as xr - - from satpy import Scene scn = Scene() start_time = datetime(2018, 5, 30, 10, 0) end_time = datetime(2018, 5, 30, 10, 15) @@ -134,9 +132,6 @@ def test_save_array_coords(self): def test_save_dataset_a_digit(self): """Test saving an array to netcdf/cf where dataset name starting with a digit.""" - import xarray as xr - - from satpy import Scene scn = Scene() scn['1'] = xr.DataArray([1, 2, 3]) with TempFile() as filename: @@ -146,9 +141,6 @@ def test_save_dataset_a_digit(self): def test_save_dataset_a_digit_prefix(self): """Test saving an array to netcdf/cf where dataset name starting with a digit with prefix.""" - import xarray as xr - - from satpy import Scene scn = Scene() scn['1'] = xr.DataArray([1, 2, 3]) with TempFile() as filename: @@ -158,9 +150,6 @@ def test_save_dataset_a_digit_prefix(self): def test_save_dataset_a_digit_prefix_include_attr(self): """Test saving an array to netcdf/cf where dataset name starting with a digit with prefix include orig name.""" - import xarray as xr - - from satpy import Scene scn = Scene() scn['1'] = xr.DataArray([1, 2, 3]) with TempFile() as filename: @@ -171,9 +160,6 @@ def test_save_dataset_a_digit_prefix_include_attr(self): def test_save_dataset_a_digit_no_prefix_include_attr(self): """Test saving an array to netcdf/cf dataset name starting with a digit with no prefix include orig name.""" - import xarray as xr - - from satpy import Scene scn = Scene() scn['1'] = xr.DataArray([1, 2, 3]) with TempFile() as filename: @@ -184,9 +170,6 @@ def test_save_dataset_a_digit_no_prefix_include_attr(self): def test_ancillary_variables(self): """Test ancillary_variables cited each other.""" - import xarray as xr - - from satpy import Scene from satpy.tests.utils import make_dataid scn = Scene() start_time = datetime(2018, 5, 30, 10, 0) @@ -209,10 +192,6 @@ def test_ancillary_variables(self): def test_groups(self): """Test creating a file with groups.""" - import xarray as xr - - from satpy import Scene - tstart = datetime(2019, 4, 1, 12, 0) tend = datetime(2019, 4, 1, 12, 15) @@ -267,9 +246,6 @@ def test_groups(self): def test_single_time_value(self): """Test setting a single time value.""" - import xarray as xr - - from satpy import Scene scn = Scene() start_time = datetime(2018, 5, 30, 10, 0) end_time = datetime(2018, 5, 30, 10, 15) @@ -288,9 +264,6 @@ def test_single_time_value(self): def test_time_coordinate_on_a_swath(self): """Test that time dimension is not added on swath data with time already as a coordinate.""" - import xarray as xr - - from satpy import Scene scn = Scene() test_array = np.array([[1, 2], [3, 4], [5, 6], [7, 8]]) times = np.array(['2018-05-30T10:05:00', '2018-05-30T10:05:01', @@ -306,9 +279,6 @@ def test_time_coordinate_on_a_swath(self): def test_bounds(self): """Test setting time bounds.""" - import xarray as xr - - from satpy import Scene scn = Scene() start_time = datetime(2018, 5, 30, 10, 0) end_time = datetime(2018, 5, 30, 10, 15) @@ -340,9 +310,6 @@ def test_bounds(self): def test_bounds_minimum(self): """Test minimum bounds.""" - import xarray as xr - - from satpy import Scene scn = Scene() start_timeA = datetime(2018, 5, 30, 10, 0) # expected to be used end_timeA = datetime(2018, 5, 30, 10, 20) @@ -368,9 +335,6 @@ def test_bounds_minimum(self): def test_bounds_missing_time_info(self): """Test time bounds generation in case of missing time.""" - import xarray as xr - - from satpy import Scene scn = Scene() start_timeA = datetime(2018, 5, 30, 10, 0) end_timeA = datetime(2018, 5, 30, 10, 15) @@ -392,9 +356,6 @@ def test_bounds_missing_time_info(self): def test_unlimited_dims_kwarg(self): """Test specification of unlimited dimensions.""" - import xarray as xr - - from satpy import Scene scn = Scene() start_time = datetime(2018, 5, 30, 10, 0) end_time = datetime(2018, 5, 30, 10, 15) @@ -411,9 +372,6 @@ def test_unlimited_dims_kwarg(self): def test_header_attrs(self): """Check global attributes are set.""" - import xarray as xr - - from satpy import Scene scn = Scene() start_time = datetime(2018, 5, 30, 10, 0) end_time = datetime(2018, 5, 30, 10, 15) @@ -546,8 +504,6 @@ def assertDictWithArraysEqual(self, d1, d2): def test_encode_attrs_nc(self): """Test attributes encoding.""" - import json - from satpy.writers.cf_writer import encode_attrs_nc attrs, expected, _ = self.get_test_attrs() @@ -567,8 +523,6 @@ def test_encode_attrs_nc(self): def test_da2cf(self): """Test the conversion of a DataArray to a CF-compatible DataArray.""" - import xarray as xr - from satpy.writers.cf_writer import CFWriter # Create set of test attributes @@ -607,8 +561,6 @@ def test_da2cf(self): def test_da2cf_one_dimensional_array(self): """Test the conversion of an 1d DataArray to a CF-compatible DataArray.""" - import xarray as xr - from satpy.writers.cf_writer import CFWriter arr = xr.DataArray(np.array([1, 2, 3, 4]), attrs={}, dims=('y',), @@ -618,10 +570,8 @@ def test_da2cf_one_dimensional_array(self): @mock.patch('satpy.writers.cf_writer.CFWriter.__init__', return_value=None) def test_collect_datasets(self, *mocks): """Test collecting CF datasets from a DataArray objects.""" - import pyresample.geometry - import xarray as xr - from satpy.writers.cf_writer import CFWriter + geos = pyresample.geometry.AreaDefinition( area_id='geos', description='geos', @@ -664,8 +614,6 @@ def test_collect_datasets(self, *mocks): def test_assert_xy_unique(self): """Test that the x and y coordinates are unique.""" - import xarray as xr - from satpy.writers.cf_writer import assert_xy_unique dummy = [[1, 2], [3, 4]] @@ -679,9 +627,6 @@ def test_assert_xy_unique(self): def test_link_coords(self): """Check that coordinates link has been established correctly.""" - import numpy as np - import xarray as xr - from satpy.writers.cf_writer import link_coords data = [[1, 2], [3, 4]] @@ -717,8 +662,6 @@ def test_link_coords(self): def test_make_alt_coords_unique(self): """Test that created coordinate variables are unique.""" - import xarray as xr - from satpy.writers.cf_writer import make_alt_coords_unique data = [[1, 2], [3, 4]] @@ -765,9 +708,6 @@ def test_make_alt_coords_unique(self): def test_area2cf(self): """Test the conversion of an area to CF standards.""" - import pyresample.geometry - import xarray as xr - from satpy.writers.cf_writer import area2cf ds_base = xr.DataArray(data=[[1, 2], [3, 4]], dims=('y', 'x'), coords={'y': [1, 2], 'x': [3, 4]}, @@ -814,9 +754,6 @@ def test_area2cf(self): def test_area2gridmapping(self): """Test the conversion from pyresample area object to CF grid mapping.""" - import pyresample.geometry - import xarray as xr - from satpy.writers.cf_writer import area2gridmapping def _gm_matches(gmapping, expected): @@ -1001,10 +938,6 @@ def _gm_matches(gmapping, expected): def test_area2lonlat(self): """Test the conversion from areas to lon/lat.""" - import dask.array as da - import pyresample.geometry - import xarray as xr - from satpy.writers.cf_writer import area2lonlat area = pyresample.geometry.AreaDefinition( @@ -1070,9 +1003,6 @@ def test_load_module_with_old_pyproj(self): def test_global_attr_default_history_and_Conventions(self): """Test saving global attributes history and Conventions.""" - import xarray as xr - - from satpy import Scene scn = Scene() start_time = datetime(2018, 5, 30, 10, 0) end_time = datetime(2018, 5, 30, 10, 15) @@ -1089,9 +1019,6 @@ def test_global_attr_default_history_and_Conventions(self): def test_global_attr_history_and_Conventions(self): """Test saving global attributes history and Conventions.""" - import xarray as xr - - from satpy import Scene scn = Scene() start_time = datetime(2018, 5, 30, 10, 0) end_time = datetime(2018, 5, 30, 10, 15) @@ -1113,9 +1040,6 @@ def test_global_attr_history_and_Conventions(self): def test_lonlat_storage(tmp_path): """Test correct storage for area with lon/lat units.""" - import xarray as xr - from pyresample import create_area_def - from ..utils import make_fake_scene scn = make_fake_scene( {"ketolysis": np.arange(25).reshape(5, 5)}, @@ -1137,9 +1061,6 @@ def test_lonlat_storage(tmp_path): def test_da2cf_lonlat(): """Test correct da2cf encoding for area with lon/lat units.""" - import xarray as xr - from pyresample import create_area_def - from satpy.resample import add_crs_xy_coords from satpy.writers.cf_writer import CFWriter @@ -1157,8 +1078,6 @@ def test_da2cf_lonlat(): def test_is_projected(caplog): """Tests for private _is_projected function.""" - import xarray as xr - from satpy.writers.cf_writer import CFWriter # test case with units but no area @@ -1189,8 +1108,6 @@ class TestCFWriterData(unittest.TestCase): def setUp(self): """Create some test data.""" - import pyresample.geometry - import xarray as xr data = [[75, 2], [3, 4]] y = [1, 2] x = [1, 2] @@ -1242,6 +1159,7 @@ def test_collect_datasets_with_latitude_named_lat(self, *mocks): from operator import getitem from satpy.writers.cf_writer import CFWriter + self.datasets_list = [self.datasets[key] for key in self.datasets] self.datasets_list_no_latlon = [self.datasets[key] for key in ['var1', 'var2']] @@ -1264,7 +1182,6 @@ class EncodingUpdateTest(unittest.TestCase): def setUp(self): """Create fake data for testing.""" - import xarray as xr self.ds = xr.Dataset({'foo': (('y', 'x'), [[1, 2], [3, 4]]), 'bar': (('y', 'x'), [[3, 4], [5, 6]])}, coords={'y': [1, 2],