accept numpy integers as node/edge ids - #339
Open
TeunHuijben wants to merge 1 commit into
Open
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Closes #338
SQLGraphsilently gave wrong answers when passed a numpy integer instead of a Pythonint:np.int64is not a subclass ofint, andsqlite3doesn't know the type — it falls back to the buffer protocol and binds it as an 8-byte BLOB, which never compares equal to an INTEGER column. So lookups returned "not found" instead of raising.RustWorkXGraphaccepted the same input fine, so the two backends disagreed. Since numpy integers come out of almost every numpy/polars call, callers were having to wrap ids inint()everywhere.Affected
SQLGraphpaths, all silently wrong rather than raising:has_node,has_edge,edge_id,successors/predecessors,in_degree/out_degree,filter(node_ids=...), and attribute filters likeNodeAttr("t") == np.int64(1).Changes
utils/_numpy_native.pywithis_int_like/to_native/to_native_list.SQLGraphconverts ids and filter values before they're bound as SQL parameters.isinstance(node_ids, int)scalar-vs-sequence checks now useis_int_like, so a numpy scalar isn't mistaken for a sequence.graph.nodes[id]/graph.edges[id]accept numpy integers.hasattr(x, "tolist")withto_native_list, which also handles a plain list of numpy scalars (the old form only handled arrays).Tests
test_numpy_integer_ids_*intest_graph_backends.pyrun against all three backends: 20 read paths asserted to give the same answer forintandnp.int64, plus the write and bulk-add paths. 23 of them failed before this change.