Repository: vc1492a/PyNomaly (language: python)

Issue:
Inconsistency in case of dataframe and distance matrix input
This is not a project issue, but a suggestion to put some kind of warning in the distance matrix example in the README.

There is an example of using distance matrix as input for LoOP in the README. It shows how `sklearn.neighbors.NearestNeighbors` could be used to obtain distance matrix together with index matrix. It seems that this way the matricies also contain distance measures to a point itself, resulting to zero distance for the first nearest neighbor of every point.
On the other hand internal method `_compute_distance_and_neighbor_matrix`, used when *data* argument is specified, excludes the distances to a point itself and so giving different scores on same data.
I took a look into [the test case](https://github.com/vc1492a/PyNomaly/blob/main/tests/test_loop.py#L349), which allowes difference of 0.15 in scores vector, and thus the difference between 0.45 and 0.6 is considered negligible.
I think the output metrices of `sklearn.neighbors.NearestNeighbors` should be transformed first to be consistent with the internal algorithm.
add python 3.13 test to workflows
This implements #64 after confirming Python 3.13 is supported by examining the documentation here: https://github.com/actions/setup-python


Discussion / hints:
@jnpsk thank you for identifying and noting this behavior in this issue! 

While it's not a project issue as you stated, I think it would be best to align the behavior of using your own distance matrix with that of the original Local Outlier Probabilities method, for consistency. Perhaps this can be resolved by adding one additional neighbor when using `sklearn.neighbors.NearestNeighbors` and truncating the distance vectors for each point to its closest neighbors (excluding itself). 

This fix should be included in the next version pushed out. 
@jnpsk can you please comment on the version of SciPy you have installed? After running the test case as is with 120 observations, I receive a difference of `0.0` between the approach using the internal method `_compute_distance_and_neighbor_matrix` and the method using `sklearn.neighbors.NearestNeighbors`. Could you please past your reproducible example below? 
Having defined `X_n120()` function as in test cases, I can run the example as follows without raising the assert error running on SciPy 1.9.1.

```
data = X_n120()

from sklearn.neighbors import NearestNeighbors
from PyNomaly import loop
from sklearn.utils._testing import assert_almost_equal

# generate distance and neighbor indices
neigh = NearestNeighbors(metric='euclidean')
neigh.fit(data)
d, idx = neigh.kneighbors(data, n_neighbors=10, return_distance=True)

# fit loop using data and distance matrix
clf1 = loop.LocalOutlierProbability(data, n_neighbors=10)
clf2 = loop.LocalOutlierProbability(distance_matrix=d, neighbor_matrix=idx, n_neighbors=10)

scores1 = clf1.fit().local_outlier_probabilities
scores2 = clf2.fit().local_outlier_probabilities

# compare the agreement between the results
assert_almost_equal(scores1, scores2, decimal=1)
```

What I mean is that the `d` matrix has zeros in the first column, whereas the internal method does not include distances to point itself, resulting in slight inconsitence. It is not a big deal on that dataset combined with chosen number of `n_neighbors = 10`. But if the assert checks more decimal places, or lower number of neigbors is used (3), the two scores differs significantly.




Fix patch:
diff --git a/.gitignore b/.gitignore
index b80b260..8455419 100644
--- a/.gitignore
+++ b/.gitignore
@@ -9,6 +9,7 @@ PyNomaly/loop_dev.py
 *.pyc
 *.coverage.*
 .coveragerc
+.pypirc
 
 # Byte-compiled / optimized / DLL files
 __pycache__/
diff --git a/changelog.md b/changelog.md
index 4204181..46520a8 100644
--- a/changelog.md
+++ b/changelog.md
@@ -4,6 +4,10 @@ All notable changes to PyNomaly will be documented in this Changelog.
 The format is based on [Keep a Changelog](http://keepachangelog.com/en/1.0.0/) 
 and adheres to [Semantic Versioning](http://semver.org/spec/v2.0.0.html).
 
+## 0.3.4 
+### Changed 
+- Changed source code as necessary to address a [user-reported issue](https://github.com/vc1492a/PyNomaly/issues/49), corrected in [this commit](https://github.com/vc1492a/PyNomaly/commit/bbdd12a318316ca9c7e0272a5b06909f3fc4f9b0)
+
 ## 0.3.3
 ### Changed
 - The implementation of the progress bar to support use when the number of 
diff --git a/readme.md b/readme.md
index 0abfa6a..7ba30d7 100644
--- a/readme.md
+++ b/readme.md
@@ -38,7 +38,7 @@ This Python 3 implementation uses Numpy and the formulas outlined in
 to calculate the Local Outlier Probability of each sample.
 
 ## Dependencies
-- Python 3.6 - 3.12
+- Python 3.6 - 3.13
 - numpy >= 1.16.3
 - python-utils >= 2.3.0
 - (optional) numba >= 0.45.1
@@ -281,7 +281,12 @@ PyNomaly provides the ability to specify a distance matrix so that any
 distance metric can be used (a neighbor index matrix must also be provided).
 This can be useful when wanting to use a distance other than the euclidean.
 
+Note that in order to maintain alignment with the LoOP definition of closest neighbors, 
+an additional neighbor is added when using [scikit-learn's NearestNeighbors](https://scikit-learn.org/1.5/modules/neighbors.html) since `NearestNeighbors` 
+includes the point itself when calculating the cloest neighbors (whereas the LoOP method does not include distances to point itself). 
+
 ```python
+import numpy as np
 from sklearn.neighbors import NearestNeighbors
 
 data = np.array([
@@ -293,11 +298,18 @@ data = np.array([
     [421.5, 90.3, 50.0]
 ])
 
-neigh = NearestNeighbors(n_neighbors=3, metric='hamming')
+# Generate distance and neighbor matrices
+n_neighbors = 3 # the number of neighbors according to the LoOP definition 
+neigh = NearestNeighbors(n_neighbors=n_neighbors+1, metric='hamming')
 neigh.fit(data)
 d, idx = neigh.kneighbors(data, return_distance=True)
 
-m = loop.LocalOutlierProbability(distance_matrix=d, neighbor_matrix=idx, n_neighbors=3).fit()
+# Remove self-distances - you MUST do this to preserve the same results as intended by the definition of LoOP
+indices = np.delete(indices, 0, 1)
+distances = np.delete(distances, 0, 1)
+
+# Fit and return scores
+m = loop.LocalOutlierProbability(distance_matrix=d, neighbor_matrix=idx, n_neighbors=n_neighbors+1).fit()
 scores = m.local_outlier_probabilities
 ```
 


Test patch:
diff --git a/.github/workflows/tests.yml b/.github/workflows/tests.yml
index eeb6f20..2d619ba 100644
--- a/.github/workflows/tests.yml
+++ b/.github/workflows/tests.yml
@@ -47,7 +47,7 @@ jobs:
     strategy:
       fail-fast: false
       matrix:
-        python-version: ["3.7", "3.8", "3.9", "3.10", "3.11", "3.12"]
+        python-version: ["3.7", "3.8", "3.9", "3.10", "3.11", "3.12", "3.13"]
 
     steps:
     - uses: actions/checkout@v4
diff --git a/tests/test_loop.py b/tests/test_loop.py
index ba453e9..ad97bde 100644
--- a/tests/test_loop.py
+++ b/tests/test_loop.py
@@ -790,3 +790,47 @@ def test_data_flipping() -> None:
         fit2.norm_prob_local_outlier_factor,
         decimal=6,
     )
+
+
+def test_distance_matrix_consistency(X_n120) -> None:
+    """
+    Test to ensure that the distance matrix is consistent with the neighbor
+    matrix and that the software is able to handle self-distances.
+    :return: None
+    """
+
+    neigh = NearestNeighbors(metric='euclidean')
+    neigh.fit(X_n120)
+    distances, indices = neigh.kneighbors(X_n120, n_neighbors=11, return_distance=True)
+
+    # remove the closest neighbor (its the point itself) from each row in the indices matrix and distances matrix
+    indices = np.delete(indices, 0, 1)
+    distances = np.delete(distances, 0, 1)
+
+    # Fit LoOP with and without distance matrix
+    clf_data = loop.LocalOutlierProbability(X_n120, n_neighbors=10)
+    clf_dist = loop.LocalOutlierProbability(distance_matrix=distances, neighbor_matrix=indices, n_neighbors=11)
+
+    # Attempt to retrieve scores and check types
+    scores_data = clf_data.fit().local_outlier_probabilities
+    scores_dist = clf_dist.fit().local_outlier_probabilities
+
+    # Debugging prints to investigate types and contents
+    print("Type of scores_data:", type(scores_data))
+    print("Type of scores_dist:", type(scores_dist))
+    print("Value of scores_data:", scores_data)
+    print("Value of scores_dist:", scores_dist)
+    print("Shape of scores_data:", scores_data.shape)
+    print("Shape of scores_dist:", scores_dist.shape)
+
+    # Convert to arrays if they aren't already
+    scores_data = np.array(scores_data) if not isinstance(scores_data, np.ndarray) else scores_data
+    scores_dist = np.array(scores_dist) if not isinstance(scores_dist, np.ndarray) else scores_dist
+
+    # Check shapes and types before assertion
+    assert scores_data.shape == scores_dist.shape, "Score shapes mismatch"
+    assert isinstance(scores_data, np.ndarray), "Expected scores_data to be a numpy array"
+    assert isinstance(scores_dist, np.ndarray), "Expected scores_dist to be a numpy array"
+
+    # Compare scores allowing for minor floating-point differences
+    assert_array_almost_equal(scores_data, scores_dist, decimal=10, err_msg="Inconsistent LoOP scores due to self-distances")