-
Notifications
You must be signed in to change notification settings - Fork 6
feat: grid serialization #100
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Conversation
Signed-off-by: jaapschoutenalliander <[email protected]>
Signed-off-by: jaapschoutenalliander <[email protected]>
Signed-off-by: jaapschoutenalliander <[email protected]>
Signed-off-by: jaapschoutenalliander <[email protected]>
Signed-off-by: jaapschoutenalliander <[email protected]>
Signed-off-by: jaapschoutenalliander <[email protected]>
Signed-off-by: jaapschoutenalliander <[email protected]>
Signed-off-by: jaapschoutenalliander <[email protected]>
Signed-off-by: jaapschoutenalliander <[email protected]>
Signed-off-by: jaapschoutenalliander <[email protected]>
Signed-off-by: jaapschoutenalliander <[email protected]>
Signed-off-by: jaapschoutenalliander <[email protected]>
I'd vote for |
| return path | ||
|
|
||
|
|
||
| def load_grid_from_json(path: Path, target_grid_class=None): |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
perhaps just:
| def load_grid_from_json(path: Path, target_grid_class=None): | |
| def deserialize(path: Path, target_grid_class=None): |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
For now that would work, but if we later add another serialisation option having load_grid_from... works better
| logger.warning(f"Failed to restore '{attr_name}': {e}") | ||
|
|
||
|
|
||
| def save_grid_to_json( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
perhaps just
| def save_grid_to_json( | |
| def serialize( |
| if isinstance(field_value, (int, float, str, bool)): | ||
| serialized_data[field.name] = field_value | ||
|
|
||
| if not isinstance(field_value, FancyArray) or field_value.size == 0: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I would crash here if not FancyArray for now instead of skipping. Or maybe this should be an option to skip?
| continue | ||
|
|
||
| if not issubclass(getattr(grid, attr_name).__class__, FancyArray): | ||
| setattr(grid, attr_name, attr_values) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should we check that it is the correct type as well?
Now you could set a bool for something that is typed as a string.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I now get the type from the grid and make sure to cast it to the expected type (if possible). I will also add test for sad flow
| assert loaded_grid.node.size == basic_grid.node.size | ||
| assert loaded_grid.line.size == basic_grid.line.size | ||
| assert list(loaded_grid.node.id) == list(basic_grid.node.id) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Cant we just use, with assert_array_equal from numpy.testing:
(also applicable to other tests in this file)
| assert loaded_grid.node.size == basic_grid.node.size | |
| assert loaded_grid.line.size == basic_grid.line.size | |
| assert list(loaded_grid.node.id) == list(basic_grid.node.id) | |
| assert_array_equal(loaded_grid.node.data, basic_grid.node.data) | |
| assert_array_equal(loaded_grid.line.data, basic_grid.line.data) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I updated to use the array_equal from pgm_ds where applicable
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ik mis tests voor wanneer het crasht.
Co-authored-by: Vincent Koppen <[email protected]> Signed-off-by: Jaap Schouten <[email protected]>
Signed-off-by: jaapschoutenalliander <[email protected]>
Signed-off-by: jaapschoutenalliander <[email protected]>
Signed-off-by: jaapschoutenalliander <[email protected]>
Signed-off-by: jaapschoutenalliander <[email protected]>
|
| txt_lines = f.readlines() | ||
| return TextSource(grid_class=cls).load_from_txt(*txt_lines) | ||
|
|
||
| def to_json(self, path: Path, **kwargs) -> Path: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this way, we'll need two extra methods for each serialization method we add in the future. I'd rather name this method serialize and add a method parameter once we include other serialization methods.
future example:
def serialize(self, path: Path, method: Literal["json", "msgpack", "parquet"], **kwargs):There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That sounds good
| assert loaded_grid.custom_metadata.size == 3 | ||
| np.testing.assert_array_equal(loaded_grid.custom_metadata.id, [100, 200, 300]) | ||
| np.testing.assert_array_almost_equal(loaded_grid.custom_metadata.metadata_value, [1.5, 2.5, 3.5]) | ||
| np.testing.assert_array_equal(loaded_grid.custom_metadata.category, [1, 2, 1]) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
commenting here, but it is about the uv.lock. I don't think we should expect any changes?



This pull request introduces robust JSON serialization and deserialization support for
Gridobjects, enabling interoperability and extension handling in the power grid modeling library. It adds new serialization utilities, integrates them into theGridAPI, and provides comprehensive unit tests to ensure correct roundtrips and compatibility across extended types