Fix matrix exponentiation type annotations - #15076
Conversation
…ons in matrix_exponentiation.py
for more information, see https://pre-commit.ci
|
@cclauss kindly review |
| https://www.hackerearth.com/practice/notes/matrix-exponentiation-1/ | ||
| """ | ||
|
|
||
| from __future__ import annotations |
There was a problem hiding this comment.
This is not needed on a Python 3.14-only codebase.
There was a problem hiding this comment.
ok I will ccorrect that
e90bbeb to
f79f9b7
Compare
|
@priya-sundaram-dev, your review, please. |
priya-sundaram-dev
left a comment
There was a problem hiding this comment.
Reviewed and tested locally — this is a solid, correct change. Nice work @Kanika0306.
graphs/kahns_algorithm_topo.py — the real win here. The old indegree = [0] * len(graph) silently assumed vertices were a contiguous 0..n-1 integer range, so any graph with sparse or non-zero-based labels was buggy. Switching to dict.fromkeys(graph, 0) fixes that genuine correctness bug, and the new sparse_graph / sparse_cycle doctests demonstrate it well. Bonus: deque + popleft() turns the queue from O(n) list.pop(0) into O(1). I ran the doctests plus a few extra cases and all pass:
{0:[1,2],1:[3],2:[3],3:[4,5],4:[],5:[]}→[0, 1, 2, 3, 4, 5]{0:[1],1:[2],2:[0]}(cycle) →None{10:[20],20:[]}→[10, 20]{10:[20],20:[10]}(cycle) →None{}(empty) →[]
ruff check and ruff format --check are both clean.
maths/matrix_exponentiation.py — correctly drops the duplicate module docstring and moves import timeit below the real docstring (fixes the E402 ordering). Good tidy-up.
Two small, non-blocking notes:
- Title/scope: the PR is titled "Fix matrix exponentiation type annotations", but the substantive change is the Kahn's-algorithm sparse-graph fix, and the matrix file is just import ordering (no type-annotation change). Consider retitling to something like "Fix Kahn's algorithm for non-contiguous vertex labels" so the history/changelog reflects what actually changed.
- (Optional, out of scope)
indegree[neighbor]still raisesKeyErrorif an edge points to a vertex that isn't a key ingraph— same latent assumption as before, just worth a docstring note that every referenced vertex must appear as a key.
Neither blocks merge. LGTM. 🚀
(Disclosure: I'm an AI agent; review performed by pulling the branch and running the tests/linters locally.)
Describe your change:
Fixes a
NameErrorinPython/maths/matrix_exponentiation.pycaused by self-referential type annotations in theMatrixclass.The
Matrix.__mul__method referencesMatrixin its parameter and return type annotations before the class definition has been fully evaluated. This causes Python to raiseNameError: name 'Matrix' is not definedwhen the module is imported.This change adds
from __future__ import annotationsto postpone the evaluation of type annotations, allowing the self-referentialMatrixannotations to be resolved correctly without changing the algorithm's behavior.Fixes #15075
Checklist: