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

Fix imports for hyppo >= 0.2.0 #785

Merged
merged 5 commits into from May 7, 2021
Merged

Fix imports for hyppo >= 0.2.0 #785

merged 5 commits into from May 7, 2021

Conversation

j1c
Copy link
Collaborator

@j1c j1c commented May 7, 2021

I'd like to use new hyppo features in versions >=0.2.0, and dealing with two versions of hyppo in environments is getting annoying.

Reference Issues/PRs

Fixes #659.

What does this implement/fix? Briefly explain your changes.

  • Removed much of the code that deals with metric functions since hyppo now natively supports all options in sklearn.metrics.pairwise.kernel_metrics and sklearn.metrics.pairwise.paired_distances. This way, we dont have to import the gaussian kernel function.
  • Warning were raised when pairing kernel metrics + distance-based tests or distance metrics + kernel-based tests, which was introduced in Latent distribution test size #366. This behavior was changed so that kernel metrics must be paired with kernel-based test (HSIC), and similarly for distance. Otherwise, error is thrown.
    • Reasoning is that hyppo doesn't allow this mismatch so doesn't make sense for graspologic to allow mismatches.
  • In Latent distribution test size #366, the defaults were "changed" to test=HSIC and metric=gaussian, but that was only changed in the docstring, not the actual code. Changed this back to test=dcorr and metric=euclidean in the docstring.
    • I dont have an opinion on what the actual default should be. But given that the code was never changed, I just changed the docstrings back. Perhaps outside of the scope of this PR to make this decision.
  • Sorted imports

@j1c j1c requested a review from bdpedigo May 7, 2021 01:39
@netlify
Copy link

netlify bot commented May 7, 2021

Deploy preview for graspologic ready!

Built with commit 3a5ff14

https://deploy-preview-785--graspologic.netlify.app

@bdpedigo
Copy link
Collaborator

bdpedigo commented May 7, 2021

i like dcorr/euclidean as the default

@j1c j1c merged commit 0be160f into dev May 7, 2021
@j1c j1c deleted the fix_hyppo branch May 7, 2021 18:25
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[BUG] New hyppo version breaks graspologic import
2 participants