Skip to content

Comments

Correctly persist EmInput entities in CsvFileSink#1429

Open
pierrepetersmeier wants to merge 18 commits intodevfrom
pp/#1337-eminputs-arent-written-while-persisting-a-jointgridcontainer
Open

Correctly persist EmInput entities in CsvFileSink#1429
pierrepetersmeier wants to merge 18 commits intodevfrom
pp/#1337-eminputs-arent-written-while-persisting-a-jointgridcontainer

Conversation

@pierrepetersmeier
Copy link
Contributor

Resolves #1337

@pierrepetersmeier pierrepetersmeier self-assigned this Sep 1, 2025
@pierrepetersmeier pierrepetersmeier added the bug Something isn't working label Sep 1, 2025
@pierrepetersmeier pierrepetersmeier marked this pull request as ready for review September 19, 2025 07:44
@staudtMarius
Copy link
Member

@pierrepetersmeier Could you add a test that reads in the persisted grid and compares it with the original grid? There are some test for grids without ems in GridIoIT. Maybe you could simply add some ems to this existing grid.

@danielfeismann
Copy link
Member

Is this complete and only waiting for some review? @pierrepetersmeier

@pierrepetersmeier
Copy link
Contributor Author

complete :)

…ing-a-jointgridcontainer

# Conflicts:
#	CHANGELOG.md
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.

Looks good except only little detail that may can be improved.

@danielfeismann
Copy link
Member

@pierrepetersmeier Could you add a test that reads in the persisted grid and compares it with the original grid? There are some test for grids without ems in GridIoIT. Maybe you could simply add some ems to this existing grid.

@pierrepetersmeier: is that done? Doesn't look like, to be honest :)

…use test failed with duplicate key exceptions due to shared output files
…while-persisting-a-jointgridcontainer' into pp/#1337-eminputs-arent-written-while-persisting-a-jointgridcontainer
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.

I added the legacy energyManagementUnits which somehow got lost in another PR but should be here. However now some test is failing. It would be great if you could have a look on that.

import edu.ie3.datamodel.io.processor.result.ResultEntityProcessor
import edu.ie3.datamodel.io.processor.timeseries.TimeSeriesProcessor
import edu.ie3.datamodel.io.processor.timeseries.TimeSeriesProcessorKey
import edu.ie3.datamodel.io.source.csv.CsvJointGridContainerSource
Copy link
Member

Choose a reason for hiding this comment

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

This import is not used.

Comment on lines 26 to 44
@@ -41,7 +40,7 @@ class GridIoIT extends Specification implements CsvTestDataMeta {
sinkHierarchic = new CsvFileSink(tempDirectory.toAbsolutePath(), hierarchicNamingStrategy, ",")
}

def cleanupSpec() {
Copy link
Member

Choose a reason for hiding this comment

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

What are the reason behind these changes?

thrown(FileException)
}

def "Input flat JointGridContainer with EmInput equals Output flat JointGridContainer."() {
Copy link
Member

Choose a reason for hiding this comment

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

It was really good that you added this test, since it disclosed that this is already covered by the first test in this file. So please remove this one and find out why persisting is not working yet (first test fails).

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

Labels

bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

EmInputs aren't written while persisting a JointGridContainer

3 participants