Merge branch 'master' of https://github.com/NOAA-OWP/wres into newDoc…

…kerize
NOAA-OWP · Oct 17, 2024 · ce3973e · ce3973e
2 parents 8eb336c + c488f90
commit ce3973e
Show file tree

Hide file tree

Showing 7 changed files with 81 additions and 7 deletions.
diff --git a/wres-config/src/wres/config/yaml/DeclarationFactory.java b/wres-config/src/wres/config/yaml/DeclarationFactory.java
@@ -47,6 +47,7 @@
 import org.yaml.snakeyaml.LoaderOptions;
 import org.yaml.snakeyaml.Yaml;
 import org.yaml.snakeyaml.constructor.Constructor;
+import org.yaml.snakeyaml.constructor.DuplicateKeyException;
 import org.yaml.snakeyaml.nodes.Tag;
 import org.yaml.snakeyaml.parser.ParserException;
 import org.yaml.snakeyaml.representer.Representer;
@@ -418,7 +419,7 @@ static JsonNode deserialize( String yaml ) throws IOException
             // Deserialize with Jackson now that any anchors/references are resolved
             return DESERIALIZER.readTree( resolvedYamlString );
         }
-        catch ( ScannerException | ParserException | JsonProcessingException e )
+        catch ( ScannerException | ParserException | JsonProcessingException | DuplicateKeyException e )
         {
             throw new IOException( "Failed to deserialize a YAML string.", e );
         }

diff --git a/wres-config/src/wres/config/yaml/components/Features.java b/wres-config/src/wres/config/yaml/components/Features.java
@@ -31,6 +31,7 @@ public record Features( Set<GeometryTuple> geometries, Map<GeometryTuple,Offset>
     /**
      * Sets the default values.
      * @param geometries the geometries
+     * @param offsets the offset values, such as a datum offset, if any
      */
     public Features
     {

diff --git a/wres-config/src/wres/config/yaml/components/Threshold.java b/wres-config/src/wres/config/yaml/components/Threshold.java
@@ -38,6 +38,7 @@ public record Threshold( wres.statistics.generated.Threshold threshold,
      * @param type the threshold type to help identify the declaration context
      * @param feature a feature
      * @param featureNameFrom the orientation of the data from which the named feature is taken
+     * @param generated true if this threshold was generated by the software, false if declared or read from a source
      */
     public Threshold
     {
@@ -65,6 +66,7 @@ public String toString()
                 .append( "thresholdType", this.type() )
                 .append( "featureName", featureString )
                 .append( "featureNameFrom", this.featureNameFrom() )
+                .append( "generated", this.generated() )
                 .build();
     }
 }
diff --git a/wres-config/test/wres/config/yaml/DeclarationFactoryTest.java b/wres-config/test/wres/config/yaml/DeclarationFactoryTest.java
@@ -27,6 +27,7 @@
 
 import static org.junit.jupiter.api.Assertions.assertAll;
 import static org.junit.jupiter.api.Assertions.assertEquals;
+import static org.junit.jupiter.api.Assertions.assertThrows;
 
 import wres.config.MetricConstants;
 import wres.config.yaml.components.AnalysisTimes;
@@ -2274,6 +2275,36 @@ void testDeserializeWithCovariates() throws IOException
         assertEquals( expected, actual );
     }
 
+    @Test
+    void testDeserializeThrowsExpectedExceptionWhenDuplicateKeysEncountered()
+    {
+        // GitHub issue #336
+        String yaml = """
+                observed:
+                  - some_file.csv
+                predicted:
+                  sources: another_file.csv
+                probability_thresholds: [0.01,0.1,0.5,0.9,0.95,0.99,0.995,0.999]
+                probability_thresholds: [0.01,0.1,0.5,0.9,0.95,0.99,0.995,0.999]
+                """;
+
+        assertThrows( IOException.class, () -> DeclarationFactory.from( yaml ) );
+    }
+
+    @Test
+    void testDeserializeThrowsExpectedExceptionWhenSpacingIncorrect()
+    {
+        String yaml = """
+                observed:
+                  - some_file.csv
+                predicted:
+                  sources: another_file.csv
+                          probability_thresholds: [0.01,0.1,0.5,0.9,0.95,0.99,0.995,0.999]
+                """;
+
+        assertThrows( IOException.class, () -> DeclarationFactory.from( yaml ) );
+    }
+
     @Test
     void testSerializeWithShortSources() throws IOException
     {

diff --git a/wres-io/src/wres/io/retrieving/database/EnsembleRetrieverFactory.java b/wres-io/src/wres/io/retrieving/database/EnsembleRetrieverFactory.java
@@ -154,7 +154,7 @@ public Supplier<Stream<TimeSeries<Ensemble>>> getBaselineRetriever( Set<Feature>
                            .setMeasurementUnitsCache( this.getMeasurementUnitsCache() )
                            .setProjectId( this.project.getId() )
                            .setFeatures( features )
-                           .setVariable( this.project.getRightVariable() )
+                           .setVariable( this.project.getBaselineVariable() )
                            .setDatasetOrientation( DatasetOrientation.BASELINE )
                            .setDeclaredExistingTimeScale( this.getDeclaredExistingTimeScale( baselineDataset ) )
                            .setDesiredTimeScale( this.desiredTimeScale )

diff --git a/wres-reading/src/wres/reading/wrds/geography/FeatureFiller.java b/wres-reading/src/wres/reading/wrds/geography/FeatureFiller.java
@@ -229,10 +229,11 @@ private static EvaluationDeclaration fillFeatures( EvaluationDeclaration evaluat
         // Set the features and feature groups, preserving any declared offsets, but using the new tuples
         Map<GeometryTuple, Offset> singletonOffsets = null;
         Map<GeometryTuple, Offset> groupOffsets = null;
-        if ( Objects.nonNull( evaluation.features()
-                                        .offsets() ) )
-        {
 
+        if ( Objects.nonNull( evaluation.features() )
+             && Objects.nonNull( evaluation.features()
+                                           .offsets() ) )
+        {
             singletonOffsets = FeatureFiller.getOffsets( filledSingletonFeatures, evaluation.features()
                                                                                             .offsets() );
         }
@@ -346,8 +347,9 @@ private static Set<GeometryTuple> fillSingletonFeatures( EvaluationDeclaration e
         Set<GeometryTuple> consolidatedFeatures = new HashSet<>( filledFeatures );
 
         // Add any implicitly declared features
-        if ( Objects.nonNull( featureService ) && !featureService.featureGroups()
-                                                                 .isEmpty() )
+        if ( Objects.nonNull( featureService )
+             && !featureService.featureGroups()
+                               .isEmpty() )
         {
             LOGGER.debug( "Discovered implicitly declared singleton features." );
 

diff --git a/wres-reading/test/wres/reading/wrds/geography/FeatureFillerTest.java b/wres-reading/test/wres/reading/wrds/geography/FeatureFillerTest.java
@@ -17,6 +17,7 @@
 import org.mockito.MockedStatic;
 import org.mockito.Mockito;
 
+import static org.junit.jupiter.api.Assertions.assertDoesNotThrow;
 import static org.junit.jupiter.api.Assertions.assertEquals;
 
 import wres.config.yaml.components.BaselineDataset;
@@ -26,6 +27,7 @@
 import wres.config.yaml.components.EvaluationDeclaration;
 import wres.config.yaml.components.EvaluationDeclarationBuilder;
 import wres.config.yaml.components.FeatureAuthority;
+import wres.config.yaml.components.FeatureGroups;
 import wres.config.yaml.components.FeatureServiceGroup;
 import wres.config.yaml.components.Features;
 import wres.config.yaml.components.FeaturesBuilder;
@@ -470,6 +472,41 @@ expectedTwo, new Offset( 0.0, 2.0, 0.0 ),
         }
     }
 
+    @Test
+    void testFillFeaturesDoesNotThrowNPEWhenNoFeaturesExistAndFeatureGroupExists() throws URISyntaxException
+    {
+        URI uri = new URI( "https://some_fake_uri" );
+        wres.config.yaml.components.FeatureService
+                featureService = new wres.config.yaml.components.FeatureService( uri, Set.of() );
+
+        EvaluationDeclaration evaluation
+                = FeatureFillerTest.getBoilerplateEvaluationWith( null,
+                                                                  featureService,
+                                                                  BOILERPLATE_DATASOURCE_USGS_SITE_CODE_AUTHORITY,
+                                                                  BOILERPLATE_DATASOURCE_NWS_LID_AUTHORITY,
+                                                                  null );
+
+        // Add a feature group
+        GeometryGroup group =
+                GeometryGroup.newBuilder()
+                             .addAllGeometryTuples( Set.of( GeometryTuple.newBuilder()
+                                                                         .setLeft( Geometry.newBuilder()
+                                                                                           .setName( "bar" ) )
+                                                                         .setRight( Geometry.newBuilder()
+                                                                                            .setName( "baz" ) )
+                                                                         .build() ) )
+                             .setRegionName( "AL" )
+                             .build();
+
+        FeatureGroups groups = new FeatureGroups( Set.of( group ), null );
+        EvaluationDeclaration adjusted = EvaluationDeclarationBuilder.builder( evaluation )
+                                                                     .features( null )
+                                                                     .featureGroups( groups )
+                                                                     .build();
+
+        assertDoesNotThrow( () -> FeatureFiller.fillFeatures( adjusted ) );
+    }
+
     /**
      * Generates a boilerplate evaluation declaration with the inputs.
      * @param features the features