diff --git a/iris_grib/_load_convert.py b/iris_grib/_load_convert.py index c4ea31ec..7acdfd94 100644 --- a/iris_grib/_load_convert.py +++ b/iris_grib/_load_convert.py @@ -225,40 +225,6 @@ def _hindcast_fix(forecast_time): return forecast_time -def fixup_float32_from_int32(value): - """ - Workaround for use when reading an IEEE 32-bit floating-point value - which the ECMWF GRIB API has erroneously treated as a 4-byte signed - integer. - - """ - # Convert from two's complement to sign-and-magnitude. - # NB. The bit patterns 0x00000000 and 0x80000000 will both be - # returned by the ECMWF GRIB API as an integer 0. Because they - # correspond to positive and negative zero respectively it is safe - # to treat an integer 0 as a positive zero. - if value < 0: - value = 0x80000000 - value - value_as_uint32 = np.array(value, dtype="u4") - value_as_float32 = value_as_uint32.view(dtype="f4") - return float(value_as_float32) - - -def fixup_int32_from_uint32(value): - """ - Workaround for use when reading a signed, 4-byte integer which the - ECMWF GRIB API has erroneously treated as an unsigned, 4-byte - integer. - - NB. This workaround is safe to use with values which are already - treated as signed, 4-byte integers. - - """ - if value >= 0x80000000: - value = 0x80000000 - value - return value - - ############################################################################### # # Identification Section 1 @@ -846,21 +812,16 @@ def grid_definition_template_12(section, metadata): lat = section["latitudeOfReferencePoint"] * _GRID_ACCURACY_IN_DEGREES lon = section["longitudeOfReferencePoint"] * _GRID_ACCURACY_IN_DEGREES scale = section["scaleFactorAtReferencePoint"] - # Catch bug in ECMWF GRIB API (present at 1.12.1) where the scale - # is treated as a signed, 4-byte integer. - if isinstance(scale, int): - scale = fixup_float32_from_int32(scale) CM_TO_M = 0.01 easting = section["XR"] * CM_TO_M northing = section["YR"] * CM_TO_M cs = icoord_systems.TransverseMercator(lat, lon, easting, northing, scale, geog_cs) - # Deal with bug in ECMWF GRIB API (present at 1.12.1) where these - # values are treated as unsigned, 4-byte integers. - x1 = fixup_int32_from_uint32(section["X1"]) - y1 = fixup_int32_from_uint32(section["Y1"]) - x2 = fixup_int32_from_uint32(section["X2"]) - y2 = fixup_int32_from_uint32(section["Y2"]) + # Fetch grid extents + x1 = section["X1"] + y1 = section["Y1"] + x2 = section["X2"] + y2 = section["Y2"] # Rather unhelpfully this grid definition template seems to be # overspecified, and thus open to inconsistency. But for determining diff --git a/iris_grib/_save_rules.py b/iris_grib/_save_rules.py index a47bd453..612cd9d7 100644 --- a/iris_grib/_save_rules.py +++ b/iris_grib/_save_rules.py @@ -48,65 +48,6 @@ _TIME_RANGE_UNITS_INVERTED = {val: key for key, val in _TIME_RANGE_UNITS.items()} -def fixup_float32_as_int32(value): - """ - Workaround for use when the ECMWF GRIB API treats an IEEE 32-bit - floating-point value as a signed, 4-byte integer. - - Returns the integer value which will result in the on-disk - representation corresponding to the IEEE 32-bit floating-point - value. - - """ - value_as_float32 = np.array(value, dtype="f4") - value_as_uint32 = value_as_float32.view(dtype="u4") - if value_as_uint32 >= 0x80000000: - # Convert from two's-complement to sign-and-magnitude. - # NB. Because of the silly representation of negative - # integers in GRIB2, there is no value we can pass to - # codes_set that will result in the bit pattern 0x80000000. - # But since that bit pattern corresponds to a floating - # point value of negative-zero, we can safely treat it as - # positive-zero instead. - value_as_grib_int = 0x80000000 - int(value_as_uint32) - else: - value_as_grib_int = int(value_as_uint32) - return value_as_grib_int - - -def fixup_int32_as_uint32(value): - """ - Workaround for use when the ECMWF GRIB API treats a signed, 4-byte - integer value as an unsigned, 4-byte integer. - - Returns the unsigned integer value which will result in the on-disk - representation corresponding to the signed, 4-byte integer value. - - """ - value = int(value) - if -0x7FFFFFFF <= value <= 0x7FFFFFFF: - if value < 0: - # Convert from two's-complement to sign-and-magnitude. - value = 0x80000000 - value - else: - msg = "{} out of range -2147483647 to 2147483647.".format(value) - raise ValueError(msg) - return value - - -def ensure_set_int32_value(grib, key, value): - """ - Ensure the workaround function :func:`fixup_int32_as_uint32` is applied as - necessary to problem keys. - - """ - try: - eccodes.codes_set(grib, key, value) - except eccodes.CodesInternalError: - value = fixup_int32_as_uint32(value) - eccodes.codes_set(grib, key, value) - - ############################################################################### # # Constants @@ -574,14 +515,10 @@ def grid_definition_template_12(cube, grib): eccodes.codes_set(grib, "Di", abs(x_step)) eccodes.codes_set(grib, "Dj", abs(y_step)) horizontal_grid_common(cube, grib) - - # GRIBAPI expects unsigned ints in X1, X2, Y1, Y2 but it should accept - # signed ints, so work around this. - # See https://software.ecmwf.int/issues/browse/SUP-1101 - ensure_set_int32_value(grib, "Y1", int(y_cm[0])) - ensure_set_int32_value(grib, "Y2", int(y_cm[-1])) - ensure_set_int32_value(grib, "X1", int(x_cm[0])) - ensure_set_int32_value(grib, "X2", int(x_cm[-1])) + eccodes.codes_set(grib, "Y1", int(y_cm[0])) + eccodes.codes_set(grib, "Y2", int(y_cm[-1])) + eccodes.codes_set(grib, "X1", int(x_cm[0])) + eccodes.codes_set(grib, "X2", int(x_cm[-1])) # Lat and lon of reference point are measured in millionths of a degree. eccodes.codes_set( @@ -603,14 +540,7 @@ def m_to_cm(value): # False easting and false northing are measured in units of (10^-2)m. eccodes.codes_set(grib, "XR", m_to_cm(cs.false_easting)) eccodes.codes_set(grib, "YR", m_to_cm(cs.false_northing)) - - # GRIBAPI expects a signed int for scaleFactorAtReferencePoint - # but it should accept a float, so work around this. - # See https://software.ecmwf.int/issues/browse/SUP-1100 value = cs.scale_factor_at_central_meridian - key_type = eccodes.codes_get_native_type(grib, "scaleFactorAtReferencePoint") - if key_type is not float: - value = fixup_float32_as_int32(value) eccodes.codes_set(grib, "scaleFactorAtReferencePoint", value) diff --git a/iris_grib/tests/unit/load_convert/test_fixup_float32_from_int32.py b/iris_grib/tests/unit/load_convert/test_fixup_float32_from_int32.py deleted file mode 100644 index e5d187b0..00000000 --- a/iris_grib/tests/unit/load_convert/test_fixup_float32_from_int32.py +++ /dev/null @@ -1,32 +0,0 @@ -# Copyright iris-grib contributors -# -# This file is part of iris-grib and is released under the BSD license. -# See LICENSE in the root of the repository for full licensing details. -""" -Unit tests for `iris_grib._load_convert.fixup_float32_from_int32`. - -""" - -# Import iris_grib.tests first so that some things can be initialised before -# importing anything else. -import iris_grib.tests as tests - -from iris_grib._load_convert import fixup_float32_from_int32 - - -class Test(tests.IrisGribTest): - def test_negative(self): - result = fixup_float32_from_int32(-0x3F000000) - self.assertEqual(result, -0.5) - - def test_zero(self): - result = fixup_float32_from_int32(0) - self.assertEqual(result, 0) - - def test_positive(self): - result = fixup_float32_from_int32(0x3F000000) - self.assertEqual(result, 0.5) - - -if __name__ == "__main__": - tests.main() diff --git a/iris_grib/tests/unit/load_convert/test_fixup_int32_from_uint32.py b/iris_grib/tests/unit/load_convert/test_fixup_int32_from_uint32.py deleted file mode 100644 index b4c3f8f0..00000000 --- a/iris_grib/tests/unit/load_convert/test_fixup_int32_from_uint32.py +++ /dev/null @@ -1,42 +0,0 @@ -# Copyright iris-grib contributors -# -# This file is part of iris-grib and is released under the BSD license. -# See LICENSE in the root of the repository for full licensing details. -""" -Unit tests for `iris_grib._load_convert.fixup_int32_from_uint32`. - -""" - -# Import iris_grib.tests first so that some things can be initialised before -# importing anything else. -import iris_grib.tests as tests - -from iris_grib._load_convert import fixup_int32_from_uint32 - - -class Test(tests.IrisGribTest): - def test_negative(self): - result = fixup_int32_from_uint32(0x80000005) - self.assertEqual(result, -5) - - def test_negative_zero(self): - result = fixup_int32_from_uint32(0x80000000) - self.assertEqual(result, 0) - - def test_zero(self): - result = fixup_int32_from_uint32(0) - self.assertEqual(result, 0) - - def test_positive(self): - result = fixup_int32_from_uint32(200000) - self.assertEqual(result, 200000) - - def test_already_negative(self): - # If we *already* have a negative value the fixup routine should - # leave it alone. - result = fixup_int32_from_uint32(-7) - self.assertEqual(result, -7) - - -if __name__ == "__main__": - tests.main() diff --git a/iris_grib/tests/unit/load_convert/test_grid_definition_template_12.py b/iris_grib/tests/unit/load_convert/test_grid_definition_template_12.py index 09d46a64..02485d14 100644 --- a/iris_grib/tests/unit/load_convert/test_grid_definition_template_12.py +++ b/iris_grib/tests/unit/load_convert/test_grid_definition_template_12.py @@ -184,17 +184,6 @@ def test_incompatible_grid_extent(self): with self.assertRaisesRegex(iris.exceptions.TranslationError, "grid"): grid_definition_template_12(section, metadata) - def test_scale_workaround(self): - section = self.section_3() - section["scaleFactorAtReferencePoint"] = 1065346526 - metadata = empty_metadata() - grid_definition_template_12(section, metadata) - expected = self.expected(0, 1) - # A float32 can't hold exactly the same value. - cs = expected["dim_coords_and_dims"][0][0].coord_system - cs.scale_factor_at_central_meridian = 0.9996012449264526 - self.assertEqual(metadata, expected) - if __name__ == "__main__": tests.main() diff --git a/iris_grib/tests/unit/save_rules/test_fixup_float32_as_int32.py b/iris_grib/tests/unit/save_rules/test_fixup_float32_as_int32.py deleted file mode 100644 index 5e576e04..00000000 --- a/iris_grib/tests/unit/save_rules/test_fixup_float32_as_int32.py +++ /dev/null @@ -1,48 +0,0 @@ -# Copyright iris-grib contributors -# -# This file is part of iris-grib and is released under the BSD license. -# See LICENSE in the root of the repository for full licensing details. -""" -Unit tests for `iris_grib._save_rules.fixup_float32_as_int32`. - -""" - -# Import iris_grib.tests first so that some things can be initialised before -# importing anything else. -import iris_grib.tests as tests - -from iris_grib._save_rules import fixup_float32_as_int32 - - -class Test(tests.IrisGribTest): - def test_positive_zero(self): - result = fixup_float32_as_int32(0.0) - self.assertEqual(result, 0) - - def test_negative_zero(self): - result = fixup_float32_as_int32(-0.0) - self.assertEqual(result, 0) - - def test_high_bit_clear_1(self): - # Start with the float32 value for the bit pattern 0x00000001. - result = fixup_float32_as_int32(1.401298464324817e-45) - self.assertEqual(result, 1) - - def test_high_bit_clear_2(self): - # Start with the float32 value for the bit pattern 0x00000002. - result = fixup_float32_as_int32(2.802596928649634e-45) - self.assertEqual(result, 2) - - def test_high_bit_set_1(self): - # Start with the float32 value for the bit pattern 0x80000001. - result = fixup_float32_as_int32(-1.401298464324817e-45) - self.assertEqual(result, -1) - - def test_high_bit_set_2(self): - # Start with the float32 value for the bit pattern 0x80000002. - result = fixup_float32_as_int32(-2.802596928649634e-45) - self.assertEqual(result, -2) - - -if __name__ == "__main__": - tests.main() diff --git a/iris_grib/tests/unit/save_rules/test_fixup_int32_as_uint32.py b/iris_grib/tests/unit/save_rules/test_fixup_int32_as_uint32.py deleted file mode 100644 index cc8e87c4..00000000 --- a/iris_grib/tests/unit/save_rules/test_fixup_int32_as_uint32.py +++ /dev/null @@ -1,40 +0,0 @@ -# Copyright iris-grib contributors -# -# This file is part of iris-grib and is released under the BSD license. -# See LICENSE in the root of the repository for full licensing details. -""" -Unit tests for `iris_grib._save_rules.fixup_int32_as_uint32`. - -""" - -# Import iris.tests first so that some things can be initialised before -# importing anything else. -import iris_grib.tests as tests - -from iris_grib._save_rules import fixup_int32_as_uint32 - - -class Test(tests.IrisGribTest): - def test_very_negative(self): - with self.assertRaises(ValueError): - fixup_int32_as_uint32(-0x80000000) - - def test_negative(self): - result = fixup_int32_as_uint32(-3) - self.assertEqual(result, 0x80000003) - - def test_zero(self): - result = fixup_int32_as_uint32(0) - self.assertEqual(result, 0) - - def test_positive(self): - result = fixup_int32_as_uint32(5) - self.assertEqual(result, 5) - - def test_very_positive(self): - with self.assertRaises(ValueError): - fixup_int32_as_uint32(0x80000000) - - -if __name__ == "__main__": - tests.main() diff --git a/iris_grib/tests/unit/save_rules/test_grid_definition_template_12.py b/iris_grib/tests/unit/save_rules/test_grid_definition_template_12.py index ad42bff6..3b841e54 100644 --- a/iris_grib/tests/unit/save_rules/test_grid_definition_template_12.py +++ b/iris_grib/tests/unit/save_rules/test_grid_definition_template_12.py @@ -105,24 +105,6 @@ def test__grid_points_approx(self): self._check_key("Di", 200) self._check_key("Dj", 500) - def test__negative_grid_points_eccodes_broken(self): - self.mock_eccodes.CodesInternalError = FakeGribError - - # Force the test to run the signed int --> unsigned int workaround. - def set(grib, key, value): - if key in ["X1", "X2", "Y1", "Y2"] and value < 0: - raise self.mock_eccodes.CodesInternalError() - grib.keys[key] = value - - self.mock_eccodes.codes_set = set - - test_cube = self._make_test_cube(x_points=[-1, 1, 3, 5, 7], y_points=[-4, 9]) - grid_definition_template_12(test_cube, self.mock_grib) - self._check_key("X1", 0x80000064) - self._check_key("X2", 700) - self._check_key("Y1", 0x80000190) - self._check_key("Y2", 900) - def test__negative_grid_points_eccodes_fixed(self): test_cube = self._make_test_cube(x_points=[-1, 1, 3, 5, 7], y_points=[-4, 9]) grid_definition_template_12(test_cube, self.mock_grib) @@ -138,25 +120,7 @@ def test__template_specifics(self): self._check_key("XR", 40000000.0) self._check_key("YR", -10000000.0) - def test__scale_factor_eccodes_broken(self): - # GRIBAPI expects a signed int for scaleFactorAtReferencePoint - # but it should accept a float, so work around this. - # See https://software.ecmwf.int/issues/browse/SUP-1100 - - def get_native_type(grib, key): - assert key == "scaleFactorAtReferencePoint" - return int - - self.mock_eccodes.codes_get_native_type = get_native_type - grid_definition_template_12(self.test_cube, self.mock_grib) - self._check_key("scaleFactorAtReferencePoint", 1065346526) - - def test__scale_factor_eccodes_fixed(self): - def get_native_type(grib, key): - assert key == "scaleFactorAtReferencePoint" - return float - - self.mock_eccodes.codes_get_native_type = get_native_type + def test__scale_factor(self): grid_definition_template_12(self.test_cube, self.mock_grib) self._check_key("scaleFactorAtReferencePoint", 0.9996012717)