fix: Remove defensive checks in bipartite graph functions - #15210
fix: Remove defensive checks in bipartite graph functions#15210tanishqraikwar83-eng wants to merge 1 commit into
Conversation
|
@priya-sundaram-dev, your review. please. Compare with: |
priya-sundaram-dev
left a comment
There was a problem hiding this comment.
Thanks for tackling #15127, @tanishqraikwar83-eng — the FIXME comments have been sitting there a while, so it's good to see someone take them on. I ran the branch, though, and unfortunately this can't merge as-is: removing the two guards breaks 13 of the existing doctests (CI will fail on check_bipatrite.py).
The reason is that the guards weren't only catching the two FIXME type/key cases — they were also making a very ordinary graph shape work: an adjacency dict where a node appears as a neighbor but is omitted as a key (i.e. a sink with no out-edges). For example the first BFS doctest:
>>> is_bipartite_bfs({0: [1, 2], 1: [0, 3], 2: [0, 4]}) # nodes 3 and 4 have no keyused to return True; with the guard gone it raises KeyError: 3. Quick repro on this branch:
$ python3 -m doctest graphs/check_bipatrite.py
... 13 failures (6 in is_bipartite_bfs, 7 in is_bipartite_dfs)
So before this can go in, the semantics question in #15127 needs a decision:
-
If a missing key should be treated as "no out-edges" (a valid graph) — which is what the current doctests assume — then the guard should stay, and the fix for the
FIXMEs is narrower: only the genuinely-invalid inputs (e.g.floatkeys used to index alist) should be allowed to raise. Deleting the guard outright is too broad. -
If the intent is that any node referenced but not keyed is an error — then those legitimate-looking doctests need to be rewritten to expect the exception (a
Traceback ...block, or# doctest: +SKIP), and the twoFIXMElines updated to match. But note that would be a behavior change users may rely on, so I'd flag it for a maintainer call rather than assume it.
My suggestion: go with (1) — keep the guards, and instead resolve the FIXMEs by normalizing the input at the top of each function (e.g. treat a missing key as []) so the "should fail" cases are handled deliberately rather than by an incidental KeyError. That keeps every current doctest green while still closing #15127.
Happy to look again once the doctests pass locally (python3 graphs/check_bipatrite.py should print All tests passed!). Thanks again for digging into this one!
— Priya Sundaram (AI agent)
fix: Remove defensive checks in bipartite graph functions
This pull request removes the defensive checks in the bipartite graph functions (both DFS and BFS) that were causing the functions to return early when a node is not in the graph. According to the FIXME comments in the code, these checks should be removed to allow natural KeyError/TypeError exceptions for invalid inputs.
Fixes #15127
Checklist: