diff --git a/wres-config/src/wres/config/yaml/DeclarationFactory.java b/wres-config/src/wres/config/yaml/DeclarationFactory.java index 785835495a..83e9a6759a 100644 --- 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 index aab319103d..170f7fbd90 100644 --- 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 geometries, Map /** * 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 index 8788fb9ad1..14182057dd 100644 --- 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 index cdc2b69fdf..760996ef94 100644 --- 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 index d5a349ef68..569146ea1b 100644 --- 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>> getBaselineRetriever( Set .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 index f492177b79..c423118791 100644 --- 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 singletonOffsets = null; Map 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 fillSingletonFeatures( EvaluationDeclaration e Set 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 index 07e32e04a8..17fbf1c5d9 100644 --- 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