Replace hopcroftkarp with scipy.sparse.csgraph.maximum_bipartite_matching

[sushovan4]

Closes #106.

Keeps to the two changes you named — the dependency and the code — plus tests.

The swap

hopcroftkarp is used in exactly two places, both in bottleneck.py. scipy.sparse.csgraph.maximum_bipartite_matching is the same algorithm, and persim already imports scipy in five modules — including scipy.sparse.csgraph itself in gromov_hausdorff.py — so this removes a dependency rather than trading one for another.

scipy was not declared; it arrived transitively through scikit-learn. It is declared now, which is correct independently of this change.

The one thing that isn't a straight substitution

The two functions return different shapes:

	returns	perfect matching test	lookup
hopcroftkarp	dict, both directions	len(res) == 2 * D.shape[0]	res["{}".format(i)]
scipy	array of column indices, -1 unmatched	not np.any(res == -1)	res[i]

The length test worked because the dict carries both sides, so 2n entries meant every row and every column was matched. D is square, so "no row unmatched" is equivalent, and that is what the code now checks.

np.inf entries behave the same under both: inf <= d is false for finite d, which is what excludes forbidden matches, and true at d = inf, so a perfect matching always exists at the largest threshold and the binary search stays well-founded.

Verification

Beyond the suite (33 passing), I ran the new implementation against the released one on 300 random diagram pairs, sizes 1–5 on each side:

• Bottleneck distances agree exactly in all 300.
• Every returned matching realises the distance it reports, under both implementations.
• The matchings are not always identical — they differ in 137 of 300.

That last point is the one worth your judgement. Where several optimal matchings exist, the two algorithms break ties differently. No trial produced more than one edge at the bottleneck cost, so the differences fall entirely among the non-binding assignments: the max edge is the same, the pairs below it are a free choice, and both make a valid one.

So the distance is unchanged, and matching=True may return a different — equally optimal — matching than before. Your existing test_matching checks that every point appears rather than pinning indices, so nothing in the suite depended on the old choice, but a downstream user asserting on exact indices would notice. Happy to add a note to the docstring saying the matching is an optimal matching rather than the one, if you'd like that stated.

Tests

Two added, both aimed at what a shape change could silently break:

• test_matching_is_exact_on_an_unambiguous_pair — pins the actual pairs on a diagram pair whose optimum is unique (cross-matching costs 1.1, the diagonal costs at least 1.0, against 0.1 for the right answer), so tie-breaking cannot mask a regression.
• test_matching_indices_form_a_permutation — asserts each side is used exactly once. An off-by-one or a misread orientation would still produce well-formed output while pairing the wrong points, and nothing in the suite would have caught it.

[codecov]

Codecov Report

❌ Patch coverage is 87.50000% with 1 line in your changes missing coverage. Please review.
✅ Project coverage is 78.85%. Comparing base (11d984a) to head (ddaa2e7).

Files with missing lines	Patch %	Lines
persim/_version.py	0.00%	1 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##           master     #108      +/-   ##
==========================================
- Coverage   79.40%   78.85%   -0.56%     
==========================================
  Files          20       20              
  Lines        1481     1480       -1     
  Branches      271      270       -1     
==========================================
- Hits         1176     1167       -9     
- Misses        229      235       +6     
- Partials       76       78       +2     

☔ View full report in Codecov by Harness.
📢 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.

[sushovan4]

Thanks for releasing the workflow run. Everything is green except codecov/project, and I do not think that one is the change — measurement below so you do not have to work it out yourself.

Running the suite on upstream/master and on this branch, same machine, same command:

                                  Stmts  Miss  Branch  BrPart   Cover   Missing
master     persim/bottleneck.py      71     1      30       2     97%   52->59, 127
this PR    persim/bottleneck.py      70     0      28       1     99%   53->60

master     TOTAL                   1481   236     542      79     82%   (108 passed)
this PR    TOTAL                   1480   235     540      78     82%   (110 passed)

bottleneck.py goes 97% to 99% — line 127 was uncovered on master and is covered here — and total missing lines go down by one rather than up. Covered-line count is unchanged.

Codecov reports the opposite: 79.40% to 78.85%, −0.56%, on 1481 to 1480 lines. That implies roughly nine covered lines lost, which does not appear locally at all. Its base is current (11d984a is master HEAD and this branch sits directly on it), so it is not a stale comparison. My guess is an upload or aggregation difference across the six matrix jobs rather than anything in the diff, but that is your infrastructure and you will read it faster than I will. Codecov's own comment agrees on the substance — "All modified and coverable lines are covered by tests" — so the failing check is the project delta being zero-tolerance rather than a coverage claim about this code.

Happy to rebase onto master and push again if a fresh run would settle it.

Two things from the description that are the actual review surface, repeated here since the run has been sitting a while:

The matching may differ from the old implementation where several optima exist. Across 300 random diagram pairs the distances agree exactly in all 300, and every returned matching realises the distance it reports under both implementations — but the matchings themselves differ in 137. No trial had more than one edge at the bottleneck cost, so the differences are entirely among non-binding assignments. Your existing test_matching checks that every point appears rather than pinning indices, so nothing in the suite depended on the old choice; a downstream user asserting exact indices would notice. I am happy to add a docstring line saying the result is an optimal matching rather than the one, if you would like that stated.

The return shape changes, so three sites move, not one. hopcroftkarp returned a dict carrying both directions, which is why the perfect-matching test was len(res) == 2 * D.shape[0]; scipy returns an array of column indices with -1 for unmatched, so the test becomes "no row unmatched" — equivalent because D is square — and the lookup becomes matching[i].

No rush from my side. Glad to make any of the above changes if you would prefer them in before merge.

[catanzaromj]

Can you also bump the minor version number?

[sushovan4]

Done — 0fab126. 0.3.8 → 0.4.0, with an entry in RELEASE.txt.

I took the minor rather than the patch on the reading that it is the behaviour change that earns it: bottleneck(..., matching=True) can return a different optimal matching than before where several exist, as measured in the description — 300/300 distances identical, 137/300 matchings different, all differences among non-binding assignments. Distances themselves are unchanged. Say the word if you meant 0.3.9 and I will move it.

Two notes while I was in there:

• RELEASE.txt's newest entry is 0.3.7, so 0.3.8 has none. That looks like it was missed when 4936e12 dropped Python 3.7 and added 3.13. I have left it alone rather than writing an entry for a release I had no part in, but it is there if you want it filled.
• The codecov/project check is still red, and it is the same delta I measured above — bottleneck.py goes 97% → 99% locally, with total missing lines down by one. Codecov's own comment still reports "All modified and coverable lines are covered by tests."

Suite is green locally: 110 passed.

[sushovan4]

Apologies — the version-bump commit carried a .venv directory with it. 5,971 files and 2.48 million insertions on your repository, which is also what broke the Read the Docs build.

Force-pushed as ddaa2e7. The commit is now the two files it should always have been:

RELEASE.txt        | 4 ++++
persim/_version.py | 2 +-

The PR as a whole is back to five files — bottleneck.py, pyproject.toml, test/test_distances.py, plus those two — and the docs build passes again. Nothing else about the change moved; 0373f72 is untouched.

The cause is worth one line of your .gitignore, because it will happen to someone else. Line 2 reads venv/, and python -m venv .venv — the form the Python docs and most tooling now use — creates .venv/, which that pattern does not match. .venv*/ would catch both, and the next variant too. Happy to send it as a separate one-line PR, or leave it with you; it is your file and not really part of this change.

On the bump itself, since it is now visible in isolation: 0.4.0 rather than 0.3.9 on the reading that the behaviour change earns the minor — bottleneck(..., matching=True) may return a different optimal matching where several exist, though the distance is identical in all 300 random pairs I measured. Still happy to move it to a patch if you meant that.

The codecov/project delta is unchanged from what I measured earlier in the thread: bottleneck.py goes 97% → 99% locally and total missing lines drop by one, so I do not believe that red is this change. Your call whether it needs anything.