Skip to content

Conversation

@pierrepetersmeier
Copy link
Contributor

Resolves #1343

@pierrepetersmeier pierrepetersmeier self-assigned this Jun 26, 2025
@danielfeismann danielfeismann added enhancement New feature or request labels Jun 26, 2025
Copy link
Member

@danielfeismann danielfeismann left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Didn't get to all of it. Just a first view thoughts that imho needs to be adapted. As well it might get easier since it would reduce the amount of changes.

@danielfeismann danielfeismann added this to the Version 8.2 milestone Jul 25, 2025
Copy link
Member

@danielfeismann danielfeismann left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This looks already quite ok, but I guess some parts can be improved.

@danielfeismann
Copy link
Member

@pierrepetersmeier it looks to me that there are some points still open.
please check my comments above on
src/test/resources/edu/ie3/datamodel/io/source/influxdb/_weather/icon/weather.txt
src/test/groovy/edu/ie3/test/common/CosmoWeatherTestData.groovy
src/test/groovy/edu/ie3/test/common/IconWeatherTestData.groovy

@pierrepetersmeier
Copy link
Contributor Author

pierrepetersmeier commented Dec 3, 2025

@danielfeismann could you please take a look? I don't understand why the four tests are still failing.deprecated

pierrepetersmeier and others added 3 commits December 3, 2025 16:53
…to-weather-data

# Conflicts:
#	src/test/groovy/edu/ie3/datamodel/io/factory/timeseries/CosmoTimeBasedWeatherValueFactoryTest.groovy
@danielfeismann
Copy link
Member

@pierrepetersmeier I don't see any failing tests in the pipeline. Maybe you can give me some details where and what is failing.

@pierrepetersmeier
Copy link
Contributor Author

It's outdated. I fixed it.

Copy link
Member

@danielfeismann danielfeismann left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for including my points. Guess we are almost there, just some minor things. Could you please also check for SonarQube issues?

@pierrepetersmeier
Copy link
Contributor Author

pierrepetersmeier commented Dec 5, 2025

"Note: In CsvWeatherSourceIconTest, the windvelocity comparison fails using Objects.equals. Appending a "5" to the decimal value did not resolve the issue. The other 3 tests have the same problem. " It's Outdated.

Copy link
Member

@danielfeismann danielfeismann left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Alright I found some more improvements, sorry for another round :)

Please note, I also found some minor improvements I directly pushed, hope this is fine for you.

@danielfeismann
Copy link
Member

@pierrepetersmeier: Please have a look to WeatherSourceTestHelper where equalsIgnoreUUID is defined.

Copy link
Member

@danielfeismann danielfeismann left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please also see my changes

Comment on lines +203 to +204
"groundTemperaturLevel1" : "8.0",
"groundTemperatureLevel2": "9.5"
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is not needed imho.

2019-08-01T16:00:00Z,13.013334274000002,287.1635211775765,241.641483540946,46.18262289142059,91.70939132296976,293.455111314491,294.9876832274387,2.2429257128168105,3.288655459909278,2.2469304566362895,3.456196177795884,3.063505298416552,0.7052856075394569,1.0173658432825086,0.6940309956521304,1.0831645239762804,0.9402700184046242,-0.009609051331189665,-0.0037207462780739073,-0.01264290152206423,-0.006439463432156433,0.9553365723370035,67776,,,,,
2019-08-01T17:00:00Z,13.015240669,306.57139450950467,180.73429610400223,49.19860365549343,175.039569078125,296.8287403584074,297.6596017457568,2.2517126616190337,3.650669504895637,2.2438962003705027,3.7980736030429303,3.339152911211416,-0.6950893529619305,-1.1085323450143223,-0.7122478653505989,-1.1293057574368208,-1.0352309009257914,0.012464921655326946,0.0059655751175761145,0.015265360298047703,0.009632113129412919,0.9553227603369525,67775,,,,,
"time","alb_rad","asob_s","aswdifd_s","aswdifu_s","aswdir_s","t_2m","tg_1","tg_2","u_10m","u_131m","u_20m","u_216m","u_65m","v_10m","v_131m","v_20m","v_216m","v_65m","w_131m","w_20m","w_216m","w_65m","z0","coordinate_id","p_131m","p_20m","p_65m","sobs_rad","t_131m"
2019-08-01T15:00:00Z,13.015240669,503.46974264373205,228.021339757131,80.8246124780934,356.2648859375,297.6241992659816,288.4101691197649,285.524199265982,2.594603775363224,3.7658971156831287,2.5812495613105044,3.941521213236469,3.4740205817325034,-0.024078646721241395,-0.029760831916596106,-0.052967885304510534,-0.009698125518755707,-0.04996610799324721,0.004091443774095653,0.0015809058504647026,0.005954484657501378,0.002666343696204668,0.9553221665631989,67775,,,,,
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

With the experience of the last puzzle, I would suggest to use also different, maybe just forward counting, temperatures also for

  • csv icon
  • sql cosmo and icon
  • txt cosmo and icon
  • json cosmo and icon

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sorry, also got confused. only for icon of course. Test should now fail on unmatched conditions also.
Same has to be done for ICON.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

enhancement New feature or request

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Add ground temperature (1m) as option to weather data

3 participants