Struct attrributes#268
Conversation
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## main #268 +/- ##
==========================================
- Coverage 87.81% 87.56% -0.25%
==========================================
Files 57 57
Lines 4998 5155 +157
Branches 877 908 +31
==========================================
+ Hits 4389 4514 +125
- Misses 384 409 +25
- Partials 225 232 +7 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
|
Maybe I should save the data field-wise in SQLGraph. Let me experiment in this way. |
|
@yfukai, let me know when this is ready for review. |
|
Thanks, it might take a while, but I'll continue working on this. |
|
@JoOkuma I believe this is good to go! Sorry for the large PR. |
|
@yfukai, I need more time to take a look at your PRs. I'll try to get them done before the end of the week. |
|
Added additional comments! It would be great if you could review this, since this changes the database structure for a "struct"-type attributes @JoOkuma |
| if isinstance(out, Attr): | ||
| out._append_field_path(name) |
There was a problem hiding this comment.
Won't this if always be true according to the return value?
There was a problem hiding this comment.
Yeah I think it's guaranteed to be Attr before this block. Thanks!
|
Sure, thanks @JoOkuma! Let me also try to reduce the complexity. |
The previous commit (db35287) mistakenly deleted upstream/main's _SQLIDSet scratch-table machinery, _create_id_scratch_table, the out_degree/copy bound-variable handling, and the three scratch-table tests — they were diffed against a stale fork main and wrongly treated as PR-added code. Restore them verbatim from main; the struct-attr column-expansion simplification from the previous commit is kept.
|
Hi @yfukai, sorry for the delayed review of this PR. There was a very complex merge with the base after the other changes. #304 This PR is extremely interesting, and I really like the approach. |
Resolve conflicts from upstream's struct-attribute PR (royerlab#268) and the single->bulk add_node/add_edge refactor: - attrs.py: take upstream's `columns` property and `f.to_attr().expr` filter reduction (handles compound AttrFilters as well as struct-field AttrComparisons). - _rustworkx_graph.py: drop the now-duplicate nested `_extract_field_path`; use upstream's module-level `_eval_filter` (supports AND/OR/XOR/NOT). - _sql_graph.py: adopt upstream's `_to_sql_clause` and the removal of the single `add_node`/`add_edge` methods (BaseGraph now delegates to the bulk variants). Mask struct flattening is preserved via `_flatten_attrs_for_write` on the bulk write paths. Mask remains stored as a struct attribute (this branch's feature); upstream left Mask as pl.Object. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
- Remove duplicate `_StructNamespace` class in attrs.py left by the upstream merge (this branch and upstream PR royerlab#268 each added an identical copy). - Drop the now-empty `if TYPE_CHECKING: pass` block and unused `TYPE_CHECKING` import in array/_graph_array.py. - Fix `_restore_pickled_column_types` docstring to reference the helper it actually uses (`_raw_binary_physical_columns`). - Align Mask.bbox_struct_fields/struct_dtype docstrings with the supported 1-to-3 dimension range. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
3 participants
Partially solves #194 by allowing
pl.Structsaved and searched in graphs.Assumes #262 merged.