From 338726902cf8e582d880c7de872211501c2b7311 Mon Sep 17 00:00:00 2001 From: Tara Drwenski Date: Fri, 27 Oct 2023 15:11:07 -0600 Subject: [PATCH 1/3] Add test to reproduce issue with reading aggregation variable that is renamed in ncml --- .../ncml/TestRenameVariableInAggregation.java | 67 +++++++++++++++++++ 1 file changed, 67 insertions(+) create mode 100644 cdm/core/src/test/java/ucar/nc2/internal/ncml/TestRenameVariableInAggregation.java diff --git a/cdm/core/src/test/java/ucar/nc2/internal/ncml/TestRenameVariableInAggregation.java b/cdm/core/src/test/java/ucar/nc2/internal/ncml/TestRenameVariableInAggregation.java new file mode 100644 index 0000000000..24a248856e --- /dev/null +++ b/cdm/core/src/test/java/ucar/nc2/internal/ncml/TestRenameVariableInAggregation.java @@ -0,0 +1,67 @@ +package ucar.nc2.internal.ncml; + +import static com.google.common.truth.Truth.assertThat; + +import java.io.IOException; +import java.io.StringReader; +import org.junit.Test; +import ucar.ma2.Array; +import ucar.ma2.InvalidRangeException; +import ucar.ma2.Section; +import ucar.nc2.NetcdfFile; +import ucar.nc2.Variable; + +public class TestRenameVariableInAggregation { + private static final double TOLERANCE = 1.0e-6; + + @Test + public void shouldRenameVariableUsedInAggregation() throws IOException, InvalidRangeException { + final String ncml = "\n" // leavit + + "\n" // leavit + + " \n" // leavit + + " \n" // leavit + + " \n" // leavit + + " \n" // leavit + + " \n" // leavit + + " \n" // leavit + + " \n" // leavit + + ""; // leavit + + checkNcmlDataset(ncml); + } + + @Test + public void shouldRenameVariableUsedInAggregationWithScan() throws IOException, InvalidRangeException { + final String ncml = "\n" // leavit + + "\n" // leavit + + " \n" // leavit + + " \n" // leavit + + " \n" // leavit + + " \n" // leavit + + " \n" // leavit + + ""; // leavit + + checkNcmlDataset(ncml); + } + + private static void checkNcmlDataset(String ncml) throws InvalidRangeException, IOException { + try (NetcdfFile ncfile = NcmlReader.readNcml(new StringReader(ncml), null, null).build()) { + final Variable oldVariable = ncfile.findVariable("T"); + assertThat((Object) oldVariable).isNull(); + final Variable newVariable = ncfile.findVariable("temperature"); + assertThat((Object) newVariable).isNotNull(); + + final Array partialArray = newVariable.read(new Section("0:2, 0:0, 0:0")); + assertThat(partialArray.getSize()).isEqualTo(3); + assertThat(partialArray.getDouble(0)).isWithin(TOLERANCE).of(0.0); + assertThat(partialArray.getDouble(1)).isWithin(TOLERANCE).of(100.0); + assertThat(partialArray.getDouble(2)).isWithin(TOLERANCE).of(200.0); + + final Array array = newVariable.read(); + assertThat(array.getSize()).isEqualTo(36); + assertThat(array.getDouble(0)).isWithin(TOLERANCE).of(0.0); + assertThat(array.getDouble(12)).isWithin(TOLERANCE).of(100.0); + assertThat(array.getDouble(24)).isWithin(TOLERANCE).of(200.0); + } + } +} From 31301e031cf2f935fe86a65c0d60b81e4b3ef4ea Mon Sep 17 00:00:00 2001 From: Tara Drwenski Date: Mon, 30 Oct 2023 11:13:11 -0600 Subject: [PATCH 2/3] Be more consistent in error handling in aggregation variable reading --- .../src/main/java/ucar/nc2/internal/ncml/AggDataset.java | 4 ++++ .../main/java/ucar/nc2/internal/ncml/AggDatasetOuter.java | 6 ++---- 2 files changed, 6 insertions(+), 4 deletions(-) diff --git a/cdm/core/src/main/java/ucar/nc2/internal/ncml/AggDataset.java b/cdm/core/src/main/java/ucar/nc2/internal/ncml/AggDataset.java index 92557b33b5..53ba1f1d84 100644 --- a/cdm/core/src/main/java/ucar/nc2/internal/ncml/AggDataset.java +++ b/cdm/core/src/main/java/ucar/nc2/internal/ncml/AggDataset.java @@ -145,6 +145,10 @@ protected Array read(Variable mainv, CancelTask cancelTask) throws IOException { return null; Variable v = findVariable(ncd, mainv); + if (v == null) { + Aggregation.logger.error("AggDataset can't find " + mainv.getFullName() + " in " + ncd.getLocation()); + throw new IllegalArgumentException("Variable '" + mainv.getFullName() + "' does not exist in aggregation."); + } if (debugRead) System.out.printf("Agg.read %s from %s in %s%n", mainv.getNameAndDimensions(), v.getNameAndDimensions(), getLocation()); diff --git a/cdm/core/src/main/java/ucar/nc2/internal/ncml/AggDatasetOuter.java b/cdm/core/src/main/java/ucar/nc2/internal/ncml/AggDatasetOuter.java index 8508dea591..697374bc94 100644 --- a/cdm/core/src/main/java/ucar/nc2/internal/ncml/AggDatasetOuter.java +++ b/cdm/core/src/main/java/ucar/nc2/internal/ncml/AggDatasetOuter.java @@ -285,10 +285,8 @@ protected Array read(Variable mainv, CancelTask cancelTask, List section) Variable v = findVariable(ncd, mainv); if (v == null) { - Aggregation.logger.error("AggOuterDimension cant find " + mainv.getFullName() + " in " + ncd.getLocation() - + "; return all zeroes!!!"); - return Array.factory(mainv.getDataType(), new Section(section).getShape()); // all zeros LOOK need missing - // value + Aggregation.logger.error("AggOuterDimension can't find " + mainv.getFullName() + " in " + ncd.getLocation()); + throw new IllegalArgumentException("Variable '" + mainv.getFullName() + "' does not exist in aggregation."); } if (Aggregation.debugRead) { From cd168566727085b0d807185b4a2bbeb288f424f8 Mon Sep 17 00:00:00 2001 From: Tara Drwenski Date: Mon, 30 Oct 2023 13:58:21 -0600 Subject: [PATCH 3/3] Add original name to existing ncml variable that may be renamed --- cdm/core/src/main/java/ucar/nc2/internal/ncml/NcmlReader.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/cdm/core/src/main/java/ucar/nc2/internal/ncml/NcmlReader.java b/cdm/core/src/main/java/ucar/nc2/internal/ncml/NcmlReader.java index 23fb0d5a06..104f7fa07a 100644 --- a/cdm/core/src/main/java/ucar/nc2/internal/ncml/NcmlReader.java +++ b/cdm/core/src/main/java/ucar/nc2/internal/ncml/NcmlReader.java @@ -989,7 +989,7 @@ private Optional readVariableExisting(Group.Builder groupBuilder, .orElseThrow(() -> new IllegalStateException("Cant find variable " + nameInFile)); } } - vb.setName(name).setDataType(dtype); + vb.setOriginalName(nameInFile).setName(name).setDataType(dtype); if (typedefS != null) { vb.setEnumTypeName(typedefS); }