8000 API pairwise_distances will require explicit V/VI param if Y is given by jnothman · Pull Request #16993 · scikit-learn/scikit-learn · GitHub
[go: up one dir, main page]
Skip to content

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

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Merged
merged 7 commits into from
Apr 27, 2020
Merged

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

merged 7 commits into from
Apr 27, 2020

Conversation

jnothman
Copy link
Member

Deprecation until version 0.25.

The current approach in _precompute_metric_params
(

scikit-learn/sklearn/metrics/pairwise.py

Lines 1429 to 1444 in f82a2cb

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.

@jnothman
API pairwise_distances will require explicit V/VI param if Y is given
e2bb381 
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
Member

Are there other metrics where we have a similar pattern?

@jnothman
Copy link
Member Author
jnothman commented Apr 22, 2020 via email

thomasjpfan
thomasjpfan reviewed Apr 26, 2020
View reviewed changes
jnothman and others added 4 commits April 27, 2020 17:48
@jnothman @thomasjpfan
Update sklearn/metrics/tests/test_pairwise.py
35ca3d0 
Co-Authored-By: Thomas J Fan <thomasjpfan@gmail.com>
@jnothman @thomasjpfan
Update sklearn/metrics/pairwise.py
97a0aea 
Co-Authored-By: Thomas J Fan <thomasjpfan@gmail.com>
@jnothman @thomasjpfan
Update sklearn/metrics/pairwise.py
11cfccc 
Co-Authored-By: Thomas J Fan <thomasjpfan@gmail.com>
@jnothman @thomasjpfan
Update sklearn/metrics/tests/test_pairwise.py
22e1fc7 
Co-Authored-By: Thomas J Fan <thomasjpfan@gmail.com>
@jnothman
Copy link
Member Author
jnothman commented Apr 27, 2020 •
edited
Loading

Good to go, @thomasjpfan?

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

tagging for inclusion #17010

adrinjalali pushed a commit that referenced this pull request Apr 30, 2020
@jnothman @thomasjpfan @adrinjalali
API pairwise_distances will require explicit V/VI param if Y is given (…
e7ad7d2 
…#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
@jnothman @thomasjpfan @gio8tisu
API pairwise_distances will require explicit V/VI param if Y is given (…
745252f 
…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
@jnothman @thomasjpfan @viclafargue
API pairwise_distances will require explicit V/VI param if Y is given (…
8d77371 
…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>
@nyanp nyanp mentioned this pull request Apr 26, 2022
Open
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@adrinjalali adrinjalali adrinjalali approved these changes

@thomasjpfan thomasjpfan thomasjpfan approved these changes

Assignees
No one assigned
Labels
module:metrics Waiting for Reviewer
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@jnothman @adrinjalali @thomasjpfan
0