Implement skip_invalid_edges argument #1337
Conversation
There was a problem hiding this comment.
Pull request overview
This PR implements the skip_invalid_edges argument for the delete_edges functionality in the ElementService, allowing users to optionally skip invalid edge deletion operations instead of failing. This addresses issue #1336.
Changes:
- Added
skip_invalid_edgesparameter todelete_edges,delete_edges_use_ti,delete_edges_use_blob, and_build_unwind_hierarchy_edges_from_blob_processmethods - Implemented logic to conditionally check edge validity using TM1's
ElementIsParentfunction before attempting deletion whenskip_invalid_edges=True - Added comprehensive test coverage for both blob and TI execution paths with valid and invalid edge scenarios
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 10 comments.
| File | Description |
|---|---|
| Tests/ElementService_test.py | Added test cases for skip_invalid_edges feature with both use_blob and use_ti options; minor formatting improvements to imports and existing code |
| TM1py/Services/ElementService.py | Implemented skip_invalid_edges parameter across delete_edges methods with conditional TI logic to check edge validity before deletion |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| h_service = self._get_hierarchy_service() | ||
| h = h_service.get(dimension_name, hierarchy_name, **kwargs) | ||
| for edge in edges: | ||
| h.remove_edge(parent=edge[0], component=edge[1]) | ||
| h_service.update(h, **kwargs) |
There was a problem hiding this comment.
The skip_invalid_edges parameter is not utilized when neither use_ti nor use_blob is specified (lines 149-153). When the default path is taken, invalid edges will still cause the operation to fail without any option to skip them. Consider either documenting this limitation or implementing skip_invalid_edges support for the default code path as well.
MariusWirtz
force-pushed
the
1336-delete-edges-skip-invalid-edges
branch
from
4542729 to
feb30f5
Compare
Fixes #1336