Conversation
|
/gemini review |
There was a problem hiding this comment.
Pull Request Overview
This PR adds table information to pipeline serialization by introducing a new Table data model and integrating it into the pipeline serialization process. The implementation includes serialization functionality, type definitions, and comprehensive test coverage.
- Adds a new
Tablemodel to represent table metadata with name, marker, target, and importance - Implements
serialize_tablesfunction to convert getML Tables objects to the new format - Updates the main pipeline serialization flow to include table data alongside existing metadata
Reviewed Changes
Copilot reviewed 11 out of 11 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
| src/getml_io/getml/tables.py | New data model defining the Table structure |
| src/getml_io/metadata/pipeline_information.py | Updates PipelineInformation model to include tables field |
| src/getml_io/serialize/pipeline.py | Implements serialize_tables function and integrates it into main pipeline serialization |
| tests/unit/types.py | Adds type definitions for TableType and TablesType |
| tests/unit/conftest.py | Adds fixtures for mock tables and updates pipeline information fixtures |
| tests/unit/serialize/test_pipeline.py | Adds unit tests for the new serialize_tables function |
| tests/unit/serialize/test_pipeline_information.py | Updates test expectations to include empty tables array |
| tests/unit/metadata/test_pipeline_information.py | Updates test expectations with table data |
| tests/integration/data/*/expected.pipeline.json | Updates integration test expectations with actual table data |
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
There was a problem hiding this comment.
Code Review
This pull request introduces serialization for table information within a pipeline. It adds a new Table data model, a serialize_tables function, and integrates this into the PipelineInformation model. The changes are well-structured and include corresponding updates to unit and integration tests. My main feedback is to add a None check in the new serialize_tables function for robustness, consistent with other serialization functions in the same file.
Adds a new `Table` data model and integrates it into the `PipelineInformation` model. This includes: - A new serialization function `serialize_tables` to convert getML `Tables` objects. - Updates to the main `serialize_pipeline` function to include table data. - Corresponding additions to unit and integration tests to cover the new functionality.
bf44654 to
c3ce193
Compare
Adds a new
Tabledata model and integrates it into thePipelineInformationmodel. This includes:serialize_tablesto convert getMLTablesobjects.serialize_pipelinefunction to include table data.