-
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
Install prophet during linux nightlies #2598
Conversation
Codecov Report
@@ Coverage Diff @@
## main #2598 +/- ##
=======================================
- Coverage 99.9% 99.9% -0.0%
=======================================
Files 295 295
Lines 27055 27059 +4
=======================================
+ Hits 27012 27014 +2
- Misses 43 45 +2
Continue to review full report at Codecov.
|
@@ -35,8 +35,12 @@ def test_all_components( | |||
n_components = 41 | |||
elif is_using_conda: | |||
n_components = 52 | |||
elif is_running_py_39_or_above or is_using_windows: | |||
elif is_using_windows and not is_running_py_39_or_above: |
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.
Test that is failing is same as #2581 so it's unrelated to this change |
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.
Looks good! Thank you :)
I'm starting to get confused by all of these different cases in test_all_components
. Do you think you could add an in-line comment stating which components we expect not to get installed so it's more clear when things are not as expected? 🙈
n_components = 53 | ||
elif is_using_windows and is_running_py_39_or_above: |
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'm starting to get confused by all of these +/-1 numbers, lol... Do you think you could add an in-line comment stating which components we expect not to get installed? 🙈
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.
Yes!
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.
This will be cleaner once sktime supports 3.9 hehe
f4d4e95
to
0febfb5
Compare
Pull Request Description
Fixes nightlies by installing prophet . They originally failed because
test_all_components
was expecting Prophet to be installed but it was missing from the build script.You can see the nightlies passing here: https://github.com/alteryx/evalml/actions/runs/1102244065
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.rst
to include this pull request by adding :pr:123
.