Skip to content

Conversation

@JoOkuma
Copy link
Member

@JoOkuma JoOkuma commented Jan 22, 2026

Depends on #244

- Add AttrSchema dataclass with defensive copying
- Create process_attr_key_args() helper to eliminate duplication
- Refactor dtype inference and SQLAlchemy mapping to use dictionaries
- Add overloaded method signatures for convenience and schema modes
- Replace _node/edge_attr_keys lists with _attr_schemas dicts
- Add comprehensive test suite (16 tests passing)

Breaking change: dtype parameter now required. Next phase will update
all calling sites throughout the codebase.
…add_edge_attr_key

This is a breaking change that requires dtype (polars.DataType) to be explicitly
specified when adding attribute keys to graphs.

Core changes:
- Add AttrSchema dataclass to store key, dtype, and default_value together
- Add utility functions: infer_default_value_from_dtype, polars_dtype_to_sqlalchemy_type,
  validate_default_value_dtype_compatibility, and process_attr_key_args
- Update BaseGraph abstract methods with overloaded signatures supporting both
  direct parameters and AttrSchema objects
- Replace _node_attr_keys/_edge_attr_keys lists with _node_attr_schemas dicts
  in RustWorkXGraph and SQLGraph
- Remove _boolean_columns tracking in SQLGraph (now uses AttrSchema)
- Fix _polars_schema_override to use == instead of isinstance for pl.Boolean check

Backend implementations:
- RustWorkXGraph: Use process_attr_key_args helper, store AttrSchema objects
- SQLGraph: Use process_attr_key_args helper, pass AttrSchema to _add_new_column
- GraphView: Delegate to root graph with all parameters preserved
- Add dtype inference fallback for complex objects that polars can't parse

Updated all callers:
- Graph methods: from_other(), match()
- Node operators: RandomNodes, RegionPropsNodes, GenericNodesOperator
- Edge operators: DistanceEdges, GenericEdgesOperator
- Solvers: ILPSolver, NearestNeighborsSolver
- IO: numpy_array, ctc
Update all test files to use the new required dtype parameter for
add_node_attr_key and add_edge_attr_key methods.

Key test fixes:
- Add dtype inference logic in test_add_node_attr_key based on value type
- Fix bbox attribute calls to use pl.Array(pl.Int64, size) instead of
  passing numpy arrays as dtype
- Add missing polars imports where needed
- Fix argument order from (key, False, dtype=pl.Boolean) to
  (key, pl.Boolean, default_value=False)
@codecov-commenter
Copy link

Codecov Report

❌ Patch coverage is 93.05556% with 20 lines in your changes missing coverage. Please review.
✅ Project coverage is 88.22%. Comparing base (37a3361) to head (3ef7d5c).
⚠️ Report is 1 commits behind head on main.

Files with missing lines Patch % Lines
src/tracksdata/graph/_graph_view.py 80.43% 6 Missing and 3 partials ⚠️
src/tracksdata/graph/_rustworkx_graph.py 94.62% 3 Missing and 2 partials ⚠️
src/tracksdata/graph/_sql_graph.py 94.33% 0 Missing and 3 partials ⚠️
src/tracksdata/utils/_dtypes.py 96.61% 1 Missing and 1 partial ⚠️
src/tracksdata/nodes/_mask.py 50.00% 0 Missing and 1 partial ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main     #246      +/-   ##
==========================================
- Coverage   88.27%   88.22%   -0.06%     
==========================================
  Files          56       56              
  Lines        4077     4237     +160     
  Branches      710      743      +33     
==========================================
+ Hits         3599     3738     +139     
- Misses        291      309      +18     
- Partials      187      190       +3     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants