-
Notifications
You must be signed in to change notification settings - Fork 87
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
Update dependencies to fix ImportError: cannot import name 'MaskedArray' from 'sklearn.utils.fixes'
errors
#1540
Conversation
Codecov Report
@@ Coverage Diff @@
## main #1540 +/- ##
=======================================
Coverage 100.0% 100.0%
=======================================
Files 232 232
Lines 16639 16639
=======================================
Hits 16631 16631
Misses 8 8 Continue to review full report at Codecov.
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think this looks good! Adding a warning to our release notes would be helpful especially since we're jumping quite a bit on our min versions. Maybe just like how we do breaking changes.
numpy>=1.19.1 | ||
pandas>=1.1.0 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Woodwork/Featuretool minimal requirements
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for fixing this @angela97lin !
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM! Agree with Freddy, if numpy and pandas are already on WW/FT requirements, do we need them on our requirements.txt as well?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@angela97lin looks good! I left some questions. Could we try not updating the min numpy version and see what happens?
I'm also curious why pandas 1.1.0 is required for woodwork (?) -- let's follow up with them on that. I wonder if they can lower the min pandas version. The more permissive we can make these ranges, the more likely people will be to use our stuff heh (certainly at the risk of bugs)
@@ -1,8 +1,8 @@ | |||
numpy>=1.16.4 | |||
pandas>=0.25.0 | |||
numpy>=1.19.1 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@angela97lin what happens if you keep this PR as-is, but delete this change so that older numpy versions are still allowed? Do the other changes still solve the problem, or is updating our min numpy also required?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@dsherry I posted in the original issue, but we get this error if we try to specifically install a version less than these because of Woodwork requirements:
ERROR: featuretools 0.22.0 has requirement pandas!=1.1.0,!=1.1.1,<2.0.0,>=0.24.1, but you'll have pandas 1.1.0 which is incompatible.
ERROR: pip's dependency resolver does not currently take into account all the packages that are installed. This behaviour is the source of the following dependency conflicts.
woodwork 0.0.6 requires numpy>=1.19.1, but you have numpy 1.16.4 which is incompatible.
ERROR: pip's dependency resolver does not currently take into account all the packages that are installed. This behaviour is the source of the following dependency conflicts.
woodwork 0.0.6 requires pandas>=1.1.0, but you have pandas 0.25.0 which is incompatible.
scipy>=1.2.1 | ||
scikit-learn>=0.23 | ||
scikit-optimize>=0.8 | ||
scikit-learn>=0.23.1 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@angela97lin so the problem was specifically in sklearn 0.23.0?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@angela97lin so the problem was specifically in sklearn 0.23.0?
Combination of scikit-learn 0.23 and scikit-optimize 0.8 and numpy: scikit-optimize/scikit-optimize#902
@dsherry Could be wrong but I don't think Woodwork can handle 0.25, because they rely so heavily on the new nullable structures (pd.NA, the new nullable float that is coming soon or was recently introduced in Pandas 1.X 🤔 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@angela97lin thanks for the details :) I filed an issue in woodwork to track lowering the numpy/pandas limits to match featuretools and evalml.
Yes I see that pd.NA
was first added in pandas 1.0.0, so perhaps the pandas version needs to stay at something like pandas>=1.1.0
. I still think the numpy version limit in woodwork is too high and that we can lower it.
Let's merge this PR, and once woodwork puts out a release with lower pip reqs for numpy and/or pandas, we can update.
Closes #1519
Repro steps:
numpy>=1.19.1
pandas>1.1.0
scikit-learn==0.23
scikit-optimize==0.8
pip install -r requirements.txt
and install pytestThis should raise the following:
Upgrading
scikit-learn==0.23.1
andscikit-optimize==0.8.1
fixes this :)--
This PR also tracks updating numpy and pandas versions. Since we rely on woodwork, we'll get errors (see issue) with our current minimal versions.