pp/#1343 Add ground temperature (1m) as option to weather data. #1348
Conversation
pierrepetersmeier
requested review from
danielfeismann
and removed request for
danielfeismann
| @@ -33,6 +36,9 @@ public class CosmoTimeBasedWeatherValueFactory extends TimeBasedWeatherValueFact | |||
| private static final String WIND_DIRECTION = "windDirection"; | |||
| private static final String WIND_VELOCITY = "windVelocity"; | |||
|
|
|||
There was a problem hiding this comment.
Please remove this empty line
| @@ -55,31 +61,58 @@ protected List<Set<String>> getFields(Class<?> entityClass) { | |||
| TEMPERATURE, | |||
| WIND_DIRECTION, | |||
| WIND_VELOCITY); | |||
|
|
|||
There was a problem hiding this comment.
Isn't it possible to simply add the additonal values here when creating the set? so the removal of the coordinate_id would be obsolete, right?
| data.getQuantity(WIND_DIRECTION, StandardUnits.WIND_DIRECTION); | ||
| ComparableQuantity<Speed> windVelocity = | ||
| data.getQuantity(WIND_VELOCITY, StandardUnits.WIND_VELOCITY); | ||
| SolarIrradianceValue solarIrradiance = new SolarIrradianceValue( |
There was a problem hiding this comment.
Please rollback this. Within Simona we expect these values to be ComparableQuantity and not SolarIrradianceValue (at least this is my understanding). Second, I would like to keep the changes in this PR limited to adding the new values and not refactoring existing code.
| Map<ComparableQuantity<Length>, TemperatureValue> groundTemperatures = new HashMap<>(); | ||
|
|
||
|
|
||
| data.getField(GROUND_TEMPERATURE_SURFACE).ifPresent(value -> { |
There was a problem hiding this comment.
What happens in the case NotPresent?
| @@ -34,6 +34,11 @@ public class IconTimeBasedWeatherValueFactory extends TimeBasedWeatherValueFacto | |||
| private static final String WIND_VELOCITY_U = "u131m"; | |||
| private static final String WIND_VELOCITY_V = "v131m"; | |||
|
|
|||
There was a problem hiding this comment.
Please also remove the line here. Please add also a getter for the new data and add them to the existing ones (where it is expected to apply). Also the buildModel seem to have it, right?
pierrepetersmeier
changed the title
pierrepetersmeier
deleted the
1343-add-ground-temperature-1m-as-option-to-weather-data
branch
Resolves #1343