Skip to content
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.

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

[FEA] UMAP / TSNE should warn when knn_graph argument is invalid #3665

Open
cjnolet opened this issue Mar 26, 2021 · 2 comments
Open

[FEA] UMAP / TSNE should warn when knn_graph argument is invalid #3665

cjnolet opened this issue Mar 26, 2021 · 2 comments
Labels
bug Something isn't working feature request New feature or request inactive-30d inactive-90d

Comments

@cjnolet
Copy link
Member

cjnolet commented Mar 26, 2021

While providing an intermediate workaround solution to a user in #1653, I made a mistake and wrote nearest_neighbors.kneighbors in a code example instead of nearest_neighbors.kneighbors_graph. However, this wasn't made immediately obvious to the user as they attempted to pass a 2-element tuple into UMAP using the knn_graph argument but it was ignored without any warning or failure.

There are a couple ways we could handle this. We should either directly support the 2-element distance/indices tuple as returned from kneighbors, we could fail, or we could warn that it's being ignored. I think there's benefits to either method, but I would be inclined to say perhaps we should consider trying to support the output as it is provided directly from kneighbors to make it easier for users and failing/warning+ignoring otherwise to make the behavior very clear.

@cjnolet cjnolet added bug Something isn't working ? - Needs Triage Need team to review and classify labels Mar 26, 2021
@cjnolet cjnolet added feature request New feature or request and removed ? - Needs Triage Need team to review and classify labels Mar 26, 2021
@github-actions
Copy link

This issue has been labeled inactive-30d due to no recent activity in the past 30 days. Please close this issue if no further response or action is needed. Otherwise, please respond with a comment indicating any updates or changes to the original issue and/or confirm this issue still needs to be addressed. This issue will be labeled inactive-90d if there is no activity in the next 60 days.

@github-actions
Copy link

This issue has been labeled inactive-90d due to no recent activity in the past 90 days. Please close this issue if no further response or action is needed. Otherwise, please respond with a comment indicating any updates or changes to the original issue and/or confirm this issue still needs to be addressed.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working feature request New feature or request inactive-30d inactive-90d
Projects
None yet
Development

No branches or pull requests

1 participant