Skip to content

Conversation

@JoOkuma
Copy link
Member

@JoOkuma JoOkuma commented Dec 12, 2025

  • check and fix node_id, source_id, etc.. usage in SQLGraph
  • write tests non-default usage of node_id and source_id, etc.

@JoOkuma
Copy link
Member Author

JoOkuma commented Dec 12, 2025

@yfukai This PR is related to #228; there are a couple of points that need to be addressed.
The SQLGraph code becomes a bit more complex, and it doesn't bring much joy. I need to consider whether there's a better alternative and if this truly resolves #228

@codecov-commenter
Copy link

Codecov Report

❌ Patch coverage is 68.18182% with 7 lines in your changes missing coverage. Please review.
✅ Project coverage is 88.36%. Comparing base (daba108) to head (68dade0).

Files with missing lines Patch % Lines
src/tracksdata/graph/_sql_graph.py 66.66% 2 Missing and 2 partials ⚠️
src/tracksdata/graph/_rustworkx_graph.py 60.00% 1 Missing and 1 partial ⚠️
src/tracksdata/metrics/_visualize.py 50.00% 1 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main     #234      +/-   ##
==========================================
- Coverage   88.37%   88.36%   -0.02%     
==========================================
  Files          55       55              
  Lines        3940     3935       -5     
  Branches      690      690              
==========================================
- Hits         3482     3477       -5     
  Misses        276      276              
  Partials      182      182              

☔ 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.

@yfukai
Copy link
Contributor

yfukai commented Dec 13, 2025

Thanks! This makes sense a lot!

  • If possible, I might stick to an option to bind the field names to the graph objects themselves rather than having them globally, since one might have graphs with different column names in a single script.
  • Maybe we could also update ["z", "y", "x"].

@JoOkuma
Copy link
Member Author

JoOkuma commented Dec 23, 2025

uhm... , ok, if we bind everything to the graph, then every default attr_key must be None and check the graph's default.
Do you see another way of implementing it?

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.

3 participants