Skip to content

API pairwise_distances will require explicit V/VI param if Y is given - #16993

Merged
thomasjpfan merged 7 commits into
scikit-learn:masterfrom
jnothman:explicit_param
Apr 27, 2020
Merged

API pairwise_distances will require explicit V/VI param if Y is given#16993
thomasjpfan merged 7 commits into
scikit-learn:masterfrom
jnothman:explicit_param

Conversation

@jnothman

Copy link
Copy Markdown
Member

Deprecation until version 0.25.

The current approach in _precompute_metric_params
(

def _precompute_metric_params(X, Y, metric=None, **kwds):
"""Precompute data-derived metric parameters if not provided
"""
if metric == "seuclidean" and 'V' not in kwds:
if X is Y:
V = np.var(X, axis=0, ddof=1)
else:
V = np.var(np.vstack([X, Y]), axis=0, ddof=1)
return {'V': V}
if metric == "mahalanobis" and 'VI' not in kwds:
if X is Y:
VI = np.linalg.inv(np.cov(X.T)).T
else:
VI = np.linalg.inv(np.cov(np.vstack([X, Y]).T)).T
return {'VI': VI}
return {}
)
means that we may be applying a different metric at training and test
time. Ideally we'd have a framework for fitting a metric on some
specific training data, but in the meantime, this deprecation stops
users making mistakes.

Deprecation until version 0.25.

The current approach in `_precompute_metric_params`
(https://github.com/scikit-learn/scikit-learn/blob/f82a2cb33871a67b36150647ece1c7e56d3132bb/sklearn/metrics/pairwise.py#L1429-L1444)
means that we may be applying a different metric at training and test
time. Ideally we'd have a framework for fitting a metric on some
specific training data, but in the meantime, this deprecation stops
users making mistakes.
@adrinjalali

Copy link
Copy Markdown
Member

Are there other metrics where we have a similar pattern?

@jnothman

jnothman commented Apr 22, 2020 via email

Copy link
Copy Markdown
Member Author

Comment thread sklearn/metrics/pairwise.py Outdated
Comment thread sklearn/metrics/pairwise.py Outdated
Comment thread sklearn/metrics/tests/test_pairwise.py Outdated
Comment thread sklearn/metrics/tests/test_pairwise.py
jnothman and others added 4 commits April 27, 2020 17:48
Co-Authored-By: Thomas J Fan <thomasjpfan@gmail.com>
Co-Authored-By: Thomas J Fan <thomasjpfan@gmail.com>
Co-Authored-By: Thomas J Fan <thomasjpfan@gmail.com>
Co-Authored-By: Thomas J Fan <thomasjpfan@gmail.com>
@jnothman

jnothman commented Apr 27, 2020

Copy link
Copy Markdown
Member Author

Good to go, @thomasjpfan?

@thomasjpfan
thomasjpfan merged commit 5b2c931 into scikit-learn:master Apr 27, 2020
@adrinjalali

Copy link
Copy Markdown
Member

tagging for inclusion #17010

adrinjalali pushed a commit that referenced this pull request Apr 30, 2020
…#16993)

* API pairwise_distances will require explicit V/VI param if Y is given

Deprecation until version 0.25.

The current approach in `_precompute_metric_params`
(https://github.com/scikit-learn/scikit-learn/blob/f82a2cb33871a67b36150647ece1c7e56d3132bb/sklearn/metrics/pairwise.py#L1429-L1444)
means that we may be applying a different metric at training and test
time. Ideally we'd have a framework for fitting a metric on some
specific training data, but in the meantime, this deprecation stops
users making mistakes.

* DOC update what's new

* Update sklearn/metrics/tests/test_pairwise.py

Co-Authored-By: Thomas J Fan <thomasjpfan@gmail.com>

* Update sklearn/metrics/pairwise.py

Co-Authored-By: Thomas J Fan <thomasjpfan@gmail.com>

* Update sklearn/metrics/pairwise.py

Co-Authored-By: Thomas J Fan <thomasjpfan@gmail.com>

* Update sklearn/metrics/tests/test_pairwise.py

Co-Authored-By: Thomas J Fan <thomasjpfan@gmail.com>

Co-authored-by: Thomas J Fan <thomasjpfan@gmail.com>
gio8tisu pushed a commit to gio8tisu/scikit-learn that referenced this pull request May 15, 2020
…scikit-learn#16993)

* API pairwise_distances will require explicit V/VI param if Y is given

Deprecation until version 0.25.

The current approach in `_precompute_metric_params`
(https://github.com/scikit-learn/scikit-learn/blob/f82a2cb33871a67b36150647ece1c7e56d3132bb/sklearn/metrics/pairwise.py#L1429-L1444)
means that we may be applying a different metric at training and test
time. Ideally we'd have a framework for fitting a metric on some
specific training data, but in the meantime, this deprecation stops
users making mistakes.

* DOC update what's new

* Update sklearn/metrics/tests/test_pairwise.py

Co-Authored-By: Thomas J Fan <thomasjpfan@gmail.com>

* Update sklearn/metrics/pairwise.py

Co-Authored-By: Thomas J Fan <thomasjpfan@gmail.com>

* Update sklearn/metrics/pairwise.py

Co-Authored-By: Thomas J Fan <thomasjpfan@gmail.com>

* Update sklearn/metrics/tests/test_pairwise.py

Co-Authored-By: Thomas J Fan <thomasjpfan@gmail.com>

Co-authored-by: Thomas J Fan <thomasjpfan@gmail.com>
viclafargue pushed a commit to viclafargue/scikit-learn that referenced this pull request Jun 26, 2020
…scikit-learn#16993)

* API pairwise_distances will require explicit V/VI param if Y is given

Deprecation until version 0.25.

The current approach in `_precompute_metric_params`
(https://github.com/scikit-learn/scikit-learn/blob/f82a2cb33871a67b36150647ece1c7e56d3132bb/sklearn/metrics/pairwise.py#L1429-L1444)
means that we may be applying a different metric at training and test
time. Ideally we'd have a framework for fitting a metric on some
specific training data, but in the meantime, this deprecation stops
users making mistakes.

* DOC update what's new

* Update sklearn/metrics/tests/test_pairwise.py

Co-Authored-By: Thomas J Fan <thomasjpfan@gmail.com>

* Update sklearn/metrics/pairwise.py

Co-Authored-By: Thomas J Fan <thomasjpfan@gmail.com>

* Update sklearn/metrics/pairwise.py

Co-Authored-By: Thomas J Fan <thomasjpfan@gmail.com>

* Update sklearn/metrics/tests/test_pairwise.py

Co-Authored-By: Thomas J Fan <thomasjpfan@gmail.com>

Co-authored-by: Thomas J Fan <thomasjpfan@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants