Conversation
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #4414 +/- ##
=======================================
+ Coverage 99.7% 99.7% +0.1%
=======================================
Files 357 357
Lines 39965 39973 +8
=======================================
+ Hits 39840 39849 +9
+ Misses 125 124 -1 ☔ View full report in Codecov by Sentry. |
|
From #4314, here's the list of ones mentioned that are / have been unpinned and don't seem to cause any issues:
Here's the list of ones that cause issues (test failures):
Also, |
eccabay
left a comment
There was a problem hiding this comment.
You may need to remove the 3.8 workflows on the github side in order to merge
docs/source/release_notes.rst
Outdated
| * Reformatted files with updated black version :pr:`4395` | ||
| * Fixes | ||
| * Changes | ||
| * Drop support for Python 3.8 :pr:`4414` |
There was a problem hiding this comment.
Mega nit: drop -> dropped :D
There was a problem hiding this comment.
Also, we should probably consider this a breaking change!
| matplotlib==3.9.0 | ||
| matplotlib-inline==0.1.7 | ||
| networkx==3.1 | ||
| nlp-primitives==2.11.0 |
There was a problem hiding this comment.
Why did nlp-primitives disappear?
There was a problem hiding this comment.
If I remember correctly, latest_dependency_checker was not happy having that in there and would fail. I'll try adding it back again and seeing what happens.
There was a problem hiding this comment.
yeah it failed again when I re-added nlp-primitives back into latest-dependency
There was a problem hiding this comment.
@MichaelFu512 I think you might want to try to get to the bottom of this. I think the NaturalLanguageFeaturizer depends on nlp-primitives for the PolarityScore and DiversityScore primitives, so it seems like nlp-primitives should be included here.
There was a problem hiding this comment.
I'm not up to date on how the dependency checkers are working in EvalML, but the latest version of nlp-primitives is 2.13.0 which was released yesterday and will be installed in your CI runs. Maybe you need to update the version from 2.11.0 to 2.13.0 here instead of removing the line?
There was a problem hiding this comment.
Same error weirdly enough,
Displaying dependencies which have changed, with main on the left and the new branch on the right:
21d20
< nlp-primitives==2.13.0
Normally, if the dependency was out of date, it'd look something like:
Displaying dependencies which have changed, with main on the left and the new branch on the right:
18c18
< matplotlib==3.8.4
---
> matplotlib==3.9.0
I'll have to check why.
There was a problem hiding this comment.
So I think the reason it failed was because of this line in make_deps_diff.sh
pip freeze | grep -v "evalml.git" | grep -E "${allow_list}" > "${DEPENDENCY_FILE_PATH}"
This line compares the requirements gotten from pip freeze | grep -v "evalml.git" to the ones in allow_list (which was made earlier in the file).
allow_list lists nlp-primitives while pip freeze | grep -v "evalml.git" lists nlp_primitives. This difference makes it so that nothing even shows up in the diff since it's looking for nlp_primitives rather than nip-primitives.
I changed the line to:
(pip freeze | grep -v "evalml.git") | tr "_" "-" | grep -E "${allow_list}" > "${DEPENDENCY_FILE_PATH}"
which just converts the "_" to "-".
jeremyliweishih
left a comment
There was a problem hiding this comment.
LGTM once Becca's comments are resolved
Pull Request Description
Featuretools and woodwork (and whole lot of other libraries) are dropping support for python 3.8, which will (and is) causing a lot of test run errors.
Closes #4416
After creating the pull request: in order to pass the release_notes_updated check you will need to update the "Future Release" section of
docs/source/release_notes.rstto include this pull request by adding :pr:123.