-
-
Notifications
You must be signed in to change notification settings - Fork 18.1k
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
DEPS: Adding missing doc dependencies to environment.yml #26657
Conversation
Codecov Report
@@ Coverage Diff @@
## master #26657 +/- ##
==========================================
- Coverage 91.87% 91.87% -0.01%
==========================================
Files 174 174
Lines 50683 50683
==========================================
- Hits 46567 46564 -3
- Misses 4116 4119 +3
Continue to review full report at Codecov.
|
1 similar comment
Codecov Report
@@ Coverage Diff @@
## master #26657 +/- ##
==========================================
- Coverage 91.87% 91.87% -0.01%
==========================================
Files 174 174
Lines 50683 50683
==========================================
- Hits 46567 46564 -3
- Misses 4116 4119 +3
Continue to review full report at Codecov.
|
- isort | ||
- moto | ||
- mypy | ||
- nbconvert>=5.4.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.
I would put some comments and/or group dependencies in this file as much as possible
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 agree, didn't think about additional grouping which makes sense, but was thinking on a comment for each dependency to explain where/why is needed. Otherwise will be difficult to know when any can be removed.
May be in a separate PR, since all them need it, and it's unrelated to the changes here? I'll create an issue, we have a sprint tonight where surely someone can work on it.
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.
sure that's fine; I think grouping with a single comment is prob good enough
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 opened #26659
Hmm this might cause some issues with xlrd throwing pending deprecation warnings as defusedxml seems to be getting installed as a dependency. Seeing that locally at least let's see if it comes up in CI |
The CI is not using this file. May be you can open a issue with what you see> I didn't see any new warning in the documentation build, didn't run the tests with this environment after the change. |
For sure. I think it might be manifested in #22682 though if I see something more concrete will open an issue |
In #26648 we detected that
environment.yml
didn't contain some of the dependencies required to build the documentation.environment.yml
is what we use to build the documentation locally. Adding them here.CC: @jorisvandenbossche