From 1484abc64fcc6aed6ac01940b2ee0d12eb4938cc Mon Sep 17 00:00:00 2001 From: James Brown <64858662+james-d-brown@users.noreply.github.com> Date: Thu, 10 Oct 2024 17:26:54 +0100 Subject: [PATCH] Fix a transposition error in the retrieval of baseline time-series from a database with an incorrect (predicted) variable name, #334. --- .../database/EnsembleRetrieverFactory.java | 2 +- .../reading/wrds/geography/FeatureFiller.java | 12 +++--- .../wrds/geography/FeatureFillerTest.java | 37 +++++++++++++++++++ 3 files changed, 45 insertions(+), 6 deletions(-) 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