diff --git a/cdm/core/src/main/java/ucar/nc2/filter/ConvertMissing.java b/cdm/core/src/main/java/ucar/nc2/filter/ConvertMissing.java index 32dadf1ffb..a4071c34f4 100644 --- a/cdm/core/src/main/java/ucar/nc2/filter/ConvertMissing.java +++ b/cdm/core/src/main/java/ucar/nc2/filter/ConvertMissing.java @@ -49,7 +49,7 @@ public static ConvertMissing createFromVariable(VariableDS var) { validMax = var.convertUnsigned(validRangeAtt.getNumericValue(1), validType).doubleValue(); hasValidMin = true; hasValidMax = true; - validRangeDifferentDataType = !validType.equals(var.getDataType()); + validRangeDifferentDataType = !validType.equals(var.getOriginalDataType()); } Attribute validMinAtt = var.findAttribute(CDM.VALID_MIN); @@ -72,7 +72,10 @@ public static ConvertMissing createFromVariable(VariableDS var) { } } - if (validRangeDifferentDataType && !signedness.equals(Signedness.UNSIGNED)) { + boolean unsignedBecauseCdmAttr = var.attributes().findAttributeString(CDM.UNSIGNED, "false").equals("true"); + // skip unscaling if signedness is UNSIGNED and the CDM _Unsigned attribute is true + boolean skipUnscale = signedness.equals(Signedness.UNSIGNED) && unsignedBecauseCdmAttr; + if (validRangeDifferentDataType && !skipUnscale) { // Signal that valid range (or min/max) was specified in unpacked values, so we // need to repack those values. Only applies when the DataTypes do not match because // the variable is unsigned. diff --git a/cdm/core/src/test/java/ucar/nc2/dataset/TestScaleOffsetMissingUnsigned.java b/cdm/core/src/test/java/ucar/nc2/dataset/TestScaleOffsetMissingUnsigned.java index da6b17990c..8a9be6be94 100644 --- a/cdm/core/src/test/java/ucar/nc2/dataset/TestScaleOffsetMissingUnsigned.java +++ b/cdm/core/src/test/java/ucar/nc2/dataset/TestScaleOffsetMissingUnsigned.java @@ -5,6 +5,7 @@ package ucar.nc2.dataset; +import static com.google.common.truth.Truth.assertThat; import static java.lang.Float.NaN; import org.junit.Assert; @@ -342,4 +343,68 @@ public void testUnsignedOffsetAttribute() throws IOException, URISyntaxException Assert.assertEquals(106, var.read().getByte(0)); // -50 + 156 == 106 } } + + @Test + public void testScaleOffsetValidRangeDiffTypesOldApi() throws URISyntaxException, IOException { + File testResource = new File(getClass().getResource("testScaleOffsetMissingUnsigned.ncml").toURI()); + + try (NetcdfDataset ncd = NetcdfDataset.openDataset(testResource.getAbsolutePath(), true, null)) { + // Same as scaleOffsetValidMaxMin, but uses valid_range attribute instead of valid_min and valid_max attributes. + VariableDS var = (VariableDS) ncd.findVariable("packedUnmatchedType"); + + // Packed value of valid min, max should only be used internally to ConvertMissing, so make sure it is + // not leaking through + assertThat(var.getValidMin()).isNotWithin(0.01).of(127); + assertThat(var.getValidMax()).isNotWithin(0.01).of(129); + // Make sure unpacked values still make it through + assertThat(var.getValidMin()).isWithin(0.01).of(255); + assertThat(var.getValidMax()).isWithin(0.01).of(259); + + // This will only work if the unpacked values of valid min/max are used by + // ConvertMissing + float[] expected = new float[] {NaN, 255, 257, 259}; + float[] actual = (float[]) var.read().getStorage(); + for (int i = 0; i < actual.length; i++) { + if (var.isInvalidData(actual[i])) { + assertThat(actual[i]).isNaN(); + assertThat(expected[i]).isNaN(); + } else { + assertThat(actual[i]).isNotNaN(); + assertThat(actual[i]).isWithin(0.01f).of(expected[i]); + } + } + } + } + + @Test + public void testScaleOffsetValidRangeDiffTypes() throws URISyntaxException, IOException { + File testResource = new File(getClass().getResource("testScaleOffsetMissingUnsigned.ncml").toURI()); + + try (NetcdfDataset ncd = NetcdfDatasets.openDataset(testResource.getAbsolutePath(), true, null)) { + // Same as scaleOffsetValidMaxMin, but uses valid_range attribute instead of valid_min and valid_max attributes. + VariableDS var = (VariableDS) ncd.findVariable("packedUnmatchedType"); + + // Packed value of valid min, max should only be used internally to ConvertMissing, so make sure it is + // not leaking through + assertThat(var.getValidMin()).isNotWithin(0.01).of(127); + assertThat(var.getValidMax()).isNotWithin(0.01).of(129); + // Make sure unpacked values still make it through + assertThat(var.getValidMin()).isWithin(0.01).of(255); + assertThat(var.getValidMax()).isWithin(0.01).of(259); + + // This will only work if the unpacked values of valid min/max are used by + // ConvertMissing + float[] expected = new float[] {NaN, 255, 257, 259}; + float[] actual = (float[]) var.read().getStorage(); + for (int i = 0; i < actual.length; i++) { + if (var.isInvalidData(actual[i])) { + assertThat(actual[i]).isNaN(); + assertThat(expected[i]).isNaN(); + } else { + assertThat(actual[i]).isNotNaN(); + assertThat(actual[i]).isWithin(0.01f).of(expected[i]); + } + } + } + } } diff --git a/cdm/core/src/test/resources/ucar/nc2/dataset/testScaleOffsetMissingUnsigned.ncml b/cdm/core/src/test/resources/ucar/nc2/dataset/testScaleOffsetMissingUnsigned.ncml index 204010b06f..7ef9f158d2 100644 --- a/cdm/core/src/test/resources/ucar/nc2/dataset/testScaleOffsetMissingUnsigned.ncml +++ b/cdm/core/src/test/resources/ucar/nc2/dataset/testScaleOffsetMissingUnsigned.ncml @@ -53,4 +53,12 @@ -50 + + + + + + 126 127 128 129 +