From 09efdae85855a79ed5d595d360cc5d1eae2e72c5 Mon Sep 17 00:00:00 2001
From: Sean Arms <67096+lesserwhirls@users.noreply.github.com>
Date: Sun, 27 Sep 2026 10:21:27 -0600
Subject: [PATCH] Unsigned variable with valid range, min, max bug
PR Unidata/netcdf-java#1603 handled the case where data are packed and
the valid min, max, or range attributes were unpacked by unscaling those
values. It did not try to unscale values when the variable was unsigned.
However, we should only skip unscaling if the variable is unsigned
because the attribute _Unsigned is set to true.
---
.../java/ucar/nc2/filter/ConvertMissing.java | 7 +-
.../TestScaleOffsetMissingUnsigned.java | 65 +++++++++++++++++++
.../testScaleOffsetMissingUnsigned.ncml | 8 +++
3 files changed, 78 insertions(+), 2 deletions(-)
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
+