Conversation
Codecov Report
@@ Coverage Diff @@
## main #2244 +/- ##
=======================================
+ Coverage 99.9% 99.9% +0.1%
=======================================
Files 280 280
Lines 24288 24294 +6
=======================================
+ Hits 24260 24266 +6
Misses 28 28
Continue to review full report at Codecov.
|
|
New issue to unpin Sphinx here. This PR will just cap Sphinx version <4.0.0 for now. |
bchen1116
requested review from
ParthivNaresh,
angela97lin,
chukarsten,
freddyaboulton and
jeremyliweishih
jeremyliweishih
left a comment
jeremyliweishih
left a comment
There was a problem hiding this comment.
looks good! Might need @chukarsten or @dsherry to merge in the dependency PR first so the dependency test passes here.
freddyaboulton
left a comment
freddyaboulton
left a comment
There was a problem hiding this comment.
Thanks @bchen1116 !
angela97lin
left a comment
angela97lin
left a comment
There was a problem hiding this comment.
LGTM!
Also filed #2248 so we can automatically catch stuff like this in the future :)
freddyaboulton
left a comment
freddyaboulton
left a comment
There was a problem hiding this comment.
Thanks so much @bchen1116 !
dsherry
left a comment
dsherry
left a comment
There was a problem hiding this comment.
@bchen1116 thank you!
I approved so that we can unblock main builds. But I do want to understand why we have to add MarkupSafe to requirements.txt. I thought its only required to build the docs, not required to run evalml.
| if name == 'imbalanced-learn': | ||
| return 'imblearn' | ||
| elif name == 'MarkupSafe': | ||
| return 'markupsafe' |
There was a problem hiding this comment.
Damn that's tough. Nice patch though.
| xgboost>=0.82,<1.3.0 | ||
| catboost>=0.20 | ||
| lightgbm>=2.3.1,<3.1.0 | ||
| MarkupSafe==1.1.1 |
There was a problem hiding this comment.
@bchen1116 why add this to our requirements? Shouldn't this only be in our docs requirements?
There was a problem hiding this comment.
When I just added this to the doc requirements, we still had failing tests. I'm not sure why it happened, but adding it to requirements solved the issue
There was a problem hiding this comment.
RTD installs the docs requirements and then evalml. evalml installs MarkupSafe via ipywidgets so if we pin markupsafe in docs requirements, it'll still be overwritten when RTD installs evalml
There was a problem hiding this comment.
@freddyaboulton understood
Here is where we configure what gets installed in our RTD builds. I wonder if we now need another requirements file, which gets installed after evalml is installed, and we can move the markupsafe pin there. I'm trying that out now in a draft PR #2261
8 participants
Sphinx v4.0.0 removes support for
add_javascriptandadd_stylesheetand replaces them with other calls.We cap the version to <4.0.0 to avoid issues with cross-referencing. (depreciated API docs here)