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
Docs build: refactor markupsafe dependency pin to be outside of requirements.txt #2261
Conversation
ccc5ec5
to
a5f046d
Compare
Codecov Report
@@ Coverage Diff @@
## main #2261 +/- ##
=========================================
+ Coverage 89.5% 100.0% +10.5%
=========================================
Files 280 280
Lines 24392 24392
=========================================
+ Hits 21827 24369 +2542
+ Misses 2565 23 -2542
Continue to review full report at Codecov.
|
- method: pip | ||
path: . | ||
- requirements: docs-requirements.txt |
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 is what did it. First install evalml dependencies, then install the pinned MarkupSafe 1.1.1 (which was already done in docs/docs-requirements.txt
before this PR)
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.
It should be ok to remove the MarkupSafe requirement from docs-requirements
as well, since nbsphinx>=0.8.5
, which is present in the docs requirements, also addresses the Markupsafe issue in its own 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.
Ah thanks, will do
@@ -5,7 +5,6 @@ ipywidgets>=7.5 | |||
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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🥂
8fa17ca
to
739f41a
Compare
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, assuming things pass
Follow-up from #2244 .
Since
MarkupSafe
is only needed for the docs, and is not needed to run evalml, it shouldn't be listed in ourrequirements.txt
. So this PR moves it out of there.Thanks to @rwedge for the ultimate solution!