Add pdist compatible python function signature

[candytaco]

Make API fully compatible with scipy's pdist, and squash memory leaks

[Copilot]

🟡 Changes recommended

Unresolved critical memory-safety and exception-handling issues block approval.

Get a fresh assessment by requesting another Copilot review.

Pull request overview

Adds a SciPy-compatible Pairwise.pdist API for Euclidean and correlation distances, with optional output arrays and improved cleanup.

Changes:

• Adds and registers pdist(X, metric, *, out).
• Documents the new API and compatibility.
• Improves NumPy reference cleanup and validation.

File summaries

File	Review summary
Readme.md	Documents pdist; the RandomForest example conflicts with the stated compatibility (nit, 1 vote).
PairwiseDistances.h	Declares and registers the new pdist entry point.
PairwiseDistances.cpp	Contains unresolved zero-row memory corruption (critical, 3 votes), unchecked conversion failures that can crash (critical, 2 votes), incorrect exception returns (moderate, 2 votes), leaked indexers in multiple paths (moderate, 3 votes each), and signed-char tolower undefined behavior (moderate, 1 vote).

Review details

Suppressed comments (5)

PairwiseDistances.cpp:535

• PyArray_FROM_O can fail and return nullptr (for example when conversion or __array__ raises), but PyArray_TYPE(probe) dereferences it immediately. Check probe before reading its type and return nullptr so invalid inputs raise their original Python exception instead of crashing the interpreter.

	PyArrayObject* probe = (PyArrayObject*)PyArray_FROM_O(arg1);
	int arrayType = PyArray_TYPE(probe);
	Py_DECREF(probe);

PairwiseDistances.cpp:555

• This branch sets a Python exception and then returns None. CPython treats a non-NULL return with an active exception as a SystemError, so invalid cluster types do not reach callers as the intended ValueError; return nullptr after setting the error.

		Py_XDECREF(clusterDistances);
		PyErr_SetString(PyExc_ValueError, "Cluster solutions need to be in uint8 or uint16");
		Py_RETURN_NONE;

PairwiseDistances.cpp:515

• The new NumPy decrefs do not release the itemIndexer and solutionIndexer allocated at lines 487-488. Each successful AVX call therefore still leaks both lookup tables; delete both indexers before returning.

	Py_DECREF(clusterSolutions);
	Py_DECREF(clusterDistances);

PairwiseDistances.cpp:231

• tolower requires either EOF or an unsigned char value, but metric[i] may be a negative char for non-ASCII metric strings. An unsupported UTF-8 metric can therefore invoke undefined behavior before the intended ValueError; cast through unsigned char before calling tolower.

		metric[i] = (char)tolower(metric[i]);

Readme.md:22

• This newly added example includes RandomForest, but the new pdist entry point only accepts euclidean and correlation; the following statement that out contains the same data as pdist is therefore false for the listed random-forest function. Qualify that statement or separate the random-forest legacy API from the pdist-compatible functions.

Pairwise.GetPairwise[Euclidean|Correlation|RandomForest]Distance(data, out)

• Files reviewed: 3/3 changed files
• Comments generated: 5
• Review effort level: Lite

---

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.