8000 KDtree.valid_metrics is no longer an attribute without warning · Issue #26742 · scikit-learn/scikit-learn · GitHub
[go: up one dir, main page]
Skip to content

KDtree.valid_metrics is no longer an attribute without warning #26742

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
Closed
martinfleis opened this issue Jun 30, 2023 · 2 comments · Fixed by #26754
Closed

KDtree.valid_metrics is no longer an attribute without warning #26742

martinfleis opened this issue Jun 30, 2023 · 2 comments · Fixed by #26754
Labels
Bug Regression
Milestone
1.3.1

Comments

@martinfleis
Copy link

Describe the bug

Prior 1.3.0, KDtree.valid_metrics was an attribute returning a list. This was also mentioned in the docs as showed in
#25364 (comment). In 1.3.0, however, #25482 added a new method KDtree.valid_metrics(), therefore checks like metric in KDtree.valid_metrics does not work anymore.

From the perspective of our downstream project, this is an unexpected change of behaviour. I don't think that there's anything we can do about it now but it is a bit unfortunate.

I wanted to open the issue to ensure the team is aware of it but given the situation I don't really expect a fix, unless you have some great idea how to resolve it.

Steps/Code to Reproduce

from sklearn.neighbors import KDTree

KDTree.valid_metrics

Expected Results

['euclidean',
 'l2',
 'minkowski',
 'p',
 'manhattan',
 'cityblock',
 'l1',
 'chebyshev',
 'infinity']

We got a list directly before. Now, we have to use KDTree.valid_metrics().

Actual Results

<function KDTree.valid_metrics>

Versions

System:
    python: 3.11.4 | packaged by conda-forge | (main, Jun 10 2023, 18:08:41) [Clang 15.0.7 ]
executable: /Users/martin/mambaforge/envs/pointpats_dev/bin/python
   machine: macOS-13.4-arm64-arm-64bit

Python dependencies:
      sklearn: 1.3.0
          pip: 23.0.1
   setuptools: 67.4.0
        numpy: 1.24.4
        scipy: 1.11.0
       Cython: None
       pandas: 2.0.3
   matplotlib: 3.7.1
       joblib: 1.2.0
threadpoolctl: 3.1.0

Built with OpenMP: True

threadpoolctl info:
       user_api: blas
   internal_api: openblas
         prefix: libopenblas
       filepath: /Users/martin/mambaforge/envs/pointpats_dev/lib/libopenblas.0.dylib
        version: 0.3.21
threading_layer: openmp
   architecture: VORTEX
    num_threads: 8

       user_api: openmp
   internal_api: openmp
         prefix: libomp
       filepath: /Users/martin/mambaforge/envs/pointpats_dev/lib/libomp.dylib
        version: None
    num_threads: 8
@martinfleis martinfleis added Bug Needs Triage Issue requires triage labels Jun 30, 2023
@martinfleis martinfleis mentioned this issue Jun 30, 2023
Merged
@exs-avianello exs-avianello mentioned this issue Jul 2, 2023
Open
@ogrisel ogrisel added Regression and removed Needs Triage Issue requires triage labels Jul 3, 2023
@ogrisel
Copy link
Member
ogrisel commented Jul 3, 2023 •
edited
Loading

This is unfortunate. @jjerphan do you think we could still expose this an attribute using a property instead of a method to restore backward compat in 1.3.1?

@ogrisel ogrisel added this to the 1.3.1 milestone Jul 3, 2023
@jjerphan jjerphan mentioned this issue Jul 3, 2023
Merged
@jjerphan
Copy link
Member
jjerphan commented Jul 3, 2023 •
edited
Loading

This is indeed an unfortunate regression. IIRC, we wanted to better document the access to valid metrics, but this is in fact a API breaking change.

I think we should revert to the previous behavior and #26754 has been opened in this regard.

@piterand piterand mentioned this issue Jul 3, 2023
Merged
@stamate stamate mentioned this issue Nov 2, 2023
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
Bug Regression
Projects
None yet
Milestone
1.3.1
Development

Successfully merging a pull request may close this issue.

FIX Revert {Ball,KD}Tree.valid_metrics to public class attributes jjerphan/scikit-learn
3 participants
@ogrisel @jjerphan @martinfleis
0