Assign instead of compare when marking explicit zeros in spatio_temporal_dist_adjacency

[karpovantonme]

Reference issue (if any)

None.

What does this implement/fix?

In spatio_temporal_dist_adjacency the sparse branch of the "keep explicit zeros" loop is a comparison, not an assignment:

# Ensure we keep explicit zeros; deal with changes in SciPy
for bi, block in enumerate(blocks):
    if isinstance(block, np.ndarray):
        block[block == 0] = -np.inf
    else:
        block.data[block.data == 0] == -1   # result discarded

The dense branch one line up does assign. The sparse one has read == since the guard was added in 3545c3f (#8050, Jul 2020), so for sparse src['dist'] the explicit zeros have never actually been marked.

Additional information

I could not find a case where this changes the returned adjacency, and I do not want to claim more than I checked. With SciPy 1.13 the explicit zeros survive both the csr_array conversion and block_diag, and np.less_equal maps a stored 0 and a stored -1 to the same True, so the two branches agree today:

import numpy as np
from scipy import sparse

rows = [0, 0, 0, 1, 1, 1, 2, 2, 2]
cols = [0, 1, 2, 0, 1, 2, 0, 1, 2]
data = [0.0, 0.02, 0.5, 0.02, 0.0, 0.03, 0.5, 0.03, 0.0]
blk = sparse.csr_matrix((data, (rows, cols)), shape=(3, 3))     # 3 explicit zeros

def run(block, dist=0.1):
    e = sparse.csr_array(sparse.block_diag([sparse.csr_array(block)]).tocsr())
    e.data[:] = np.less_equal(e.data, dist)
    e.eliminate_zeros()
    return e.toarray()

marked = blk.copy()
marked.data[marked.data == 0] = -1
assert np.array_equal(run(blk), run(marked))    # passes

So this is not a behaviour fix on current SciPy. It is the line doing what the comment next to it says it does, and what the dense branch already does — the comment's "deal with changes in SciPy" is exactly the case the marking is meant to survive, and right now it would not.

Slicing does preserve the explicit zeros (src['dist'][vertno, :][:, vertno] keeps nnz including the stored 0s on SciPy 1.13), so there is something for the assignment to act on.

No changelog entry, since there is no user-facing change; happy to add one if you would rather have it recorded.

AI assistance, per the policy: the line was surfaced by a static-analysis sweep (ruff B015) run with Claude Opus in Claude Code, which also wrote the one-character patch and the SciPy check above. I reviewed the diff, the blame and the reasoning about whether it changes results before submitting.

[welcome]

Hello! 👋 Thanks for opening your first pull request here! ❤️ We will try to get back to you soon. 🚴

[larsoner]

Oof... could you add a tiny test that would have caught this? And also a docs/changes/dev/14124.bugfix.rst? And again if you just log into CircleCI once with your GitHub credentials (no need to add any projects or anything), it will allow the docs CI to run

[karpovantonme]

Thanks for looking. The changelog entry and the CircleCI login I can sort out, but I owe you an honest answer on the test first: I could not write one, because I cannot find an input where the two versions differ.

I went looking for one. Both values end up on the same side of the comparison two lines down:

edges.data[:] = np.less_equal(edges.data, dist)
edges.eliminate_zeros()

0 <= dist and -1 <= dist are both True for any non-negative dist, so the entry survives either way. I checked whether the explicit zeros get dropped somewhere in between, since that would be the obvious way for this to matter, and they do not: sparse.csr_array() keeps them, and so does sparse.block_diag, with one block or several.

def run(val, dist):
    m = sparse.csr_matrix(([val, 2., 2., val], ([0, 0, 1, 1], [0, 1, 0, 1])), shape=(3, 3))
    e = sparse.block_diag([sparse.csr_array(m)])
    e.data[:] = np.less_equal(e.data, dist)
    e = e.tocsr(); e.eliminate_zeros(); e = e.tocoo()
    return sorted(zip(e.row.tolist(), e.col.tolist()))

for dist in (0.0, 1.0, 2.0, 5.0):
    assert run(0.0, dist) == run(-1.0, dist)   # passes for all of them

So as far as I can tell the line is a no-op today, and this is a correctness cleanup rather than a bug fix: the statement does not do what it plainly reads as doing, and ruff's B015 flags it. The np.ndarray branch right above it does perform the assignment, which is what made me look in the first place.

Three ways forward, your call:

1. I add a test that pins the intent instead of the bug: explicit zeros in a sparse dist matrix must survive into the adjacency. It passes before and after, so it does not "catch" anything, but it stops the next person from removing the line as dead code.
2. You know an input where this does bite and I missed it. Tell me the shape and I will write the test around it.
3. It is not worth a changelog entry at all, in which case I am happy to close this and keep Align _AbstractRenderer.tube with the PyVista implementation #14125 as the only one.

I would rather say this than hand you a test that passes either way and call it a regression test.

[larsoner]

Okay I think we can live without a test here then. I'll start CircleCI and mark for merge-when-green, thanks in advance @karpovantonme !

[welcome]

🎉 Congrats on merging your first pull request! 🥳 Looking forward to seeing more from you in the future! 💪