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

[BUG] scipy 1.8 breaks hyppo import #303

Closed
bdpedigo opened this issue Feb 7, 2022 · 6 comments · Fixed by #304
Closed

[BUG] scipy 1.8 breaks hyppo import #303

bdpedigo opened this issue Feb 7, 2022 · 6 comments · Fixed by #304
Labels
bug Something isn't working

Comments

@bdpedigo
Copy link
Contributor

bdpedigo commented Feb 7, 2022

Reproducing code example:

import hyppo.discrim

Error message

/opt/hostedtoolcache/Python/3.8.12/x64/lib/python3.8/site-packages/hyppo/discrim/__init__.py:1: in <module>
    from .discrim_one_samp import DiscrimOneSample
/opt/hostedtoolcache/Python/3.8.12/x64/lib/python3.8/site-packages/hyppo/discrim/discrim_one_samp.py:5: in <module>
    from ._utils import _CheckInputs
/opt/hostedtoolcache/Python/3.8.12/x64/lib/python3.8/site-packages/hyppo/discrim/_utils.py:4: in <module>
    from ..tools import check_ndarray_xy, check_reps, contains_nan, convert_xy_float64
/opt/hostedtoolcache/Python/3.8.12/x64/lib/python3.8/site-packages/hyppo/tools/__init__.py:1: in <module>
    from .common import *
/opt/hostedtoolcache/Python/3.8.12/x64/lib/python3.8/site-packages/hyppo/tools/common.py:6: in <module>
    from scipy.stats.stats import _contains_nan
E   ImportError: cannot import name '_contains_nan' from 'scipy.stats.stats' (/opt/hostedtoolcache/Python/3.8.12/x64/lib/python3.8/site-packages/scipy/stats/stats.py)

The function _contains_nan() was moved to a different file in scipy 1.8. In general, this is why it's a bad idea to rely on importing private functions.

Recommend just copy-pasting the private function into hyppo explicitly.

@bdpedigo bdpedigo added the bug Something isn't working label Feb 7, 2022
@bdpedigo
Copy link
Contributor Author

bdpedigo commented Feb 7, 2022

P.S. @sampan501 would appreciate resolving this as it's breaking tests in graspologic and I'd rather not have to pin our versions for something like this

@sampan501
Copy link
Member

sure I can make a release for this once it builds

@bdpedigo
Copy link
Contributor Author

bdpedigo commented Feb 7, 2022

thanks @sampan501! LMK if you get a chance to release so we can bump our dependencies to reflect that

@bdpedigo
Copy link
Contributor Author

bdpedigo commented Feb 9, 2022

@sampan501 i know you mentioned you had some other stuff to review prior to releasing - would it be possible to release a patch with just #304 anytime soon? only reason I'm in a rush is that it's breaking everything downstream including people at MSR trying to use graspologic. if not just lmk and we'll just pin our scipy deps for now

@sampan501
Copy link
Member

I was just going to release it by the end of the week if that works? Or do you need it faster?

@bdpedigo
Copy link
Contributor Author

bdpedigo commented Feb 9, 2022

I was just going to release it by the end of the week if that works? Or do you need it faster?

oh that works, thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants