Skip to content

DOC/CI: restore travis CI doc build environment #26621

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

Merged

Conversation

jorisvandenbossche
Copy link
Member

xref #26591 (comment)

This is one option (adding back the specific doc build environment yml file), another option would be to actually update the main environment.yaml file to be suitable for the doc build.
At the moment, my preference is to add back the separate yml file because 1) those additional dependencies are not necessarily needed for a dev env setup, and just makes that env even more heavy and 2) we might want to pin to certain versions from time to time for the doc build (eg we should consider pinning sphinx to < 2.0, which is not necessarily needed for all contributors to do as well).

Another thing is that this env could actually use an update (eg python 3.6 -> 3.7)

cc @datapythonista

@jorisvandenbossche
Copy link
Member Author

  1. those additional dependencies are not necessarily needed for a dev env setup

Although I suppose it is actually the intent of environment.yaml to include all optional dependencies as well (it already includes some dependencies that are only needed for the doc build, such as statsmodels and xarray), so not having pyarrow could be regarded as an oversight.

@codecov
Copy link

codecov bot commented Jun 2, 2019

Codecov Report

Merging #26621 into master will decrease coverage by <.01%.
The diff coverage is n/a.

Impacted file tree graph

@@            Coverage Diff             @@
##           master   #26621      +/-   ##
==========================================
- Coverage   91.86%   91.85%   -0.01%     
==========================================
  Files         174      174              
  Lines       50722    50722              
==========================================
- Hits        46594    46589       -5     
- Misses       4128     4133       +5
Flag Coverage Δ
#multiple 90.39% <ø> (ø) ⬆️
#single 41.78% <ø> (-0.11%) ⬇️
Impacted Files Coverage Δ
pandas/io/gbq.py 78.94% <0%> (-10.53%) ⬇️
pandas/core/frame.py 97% <0%> (-0.12%) ⬇️
pandas/util/testing.py 90.81% <0%> (-0.11%) ⬇️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update addc5fc...5a76c70. Read the comment docs.

1 similar comment
@codecov
Copy link

codecov bot commented Jun 2, 2019

Codecov Report

Merging #26621 into master will decrease coverage by <.01%.
The diff coverage is n/a.

Impacted file tree graph

@@            Coverage Diff             @@
##           master   #26621      +/-   ##
==========================================
- Coverage   91.86%   91.85%   -0.01%     
==========================================
  Files         174      174              
  Lines       50722    50722              
==========================================
- Hits        46594    46589       -5     
- Misses       4128     4133       +5
Flag Coverage Δ
#multiple 90.39% <ø> (ø) ⬆️
#single 41.78% <ø> (-0.11%) ⬇️
Impacted Files Coverage Δ
pandas/io/gbq.py 78.94% <0%> (-10.53%) ⬇️
pandas/core/frame.py 97% <0%> (-0.12%) ⬇️
pandas/util/testing.py 90.81% <0%> (-0.11%) ⬇️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update addc5fc...5a76c70. Read the comment docs.

@simonjayhawkins simonjayhawkins added CI Continuous Integration Docs labels Jun 2, 2019
@jreback jreback added this to the 0.25.0 milestone Jun 2, 2019
@jreback
Copy link
Contributor

jreback commented Jun 2, 2019

@jorisvandenbossche looks fine, merge away when ready

@datapythonista
Copy link
Member

I guess I'm missing something. I think I added to environment.yml all the dependencies to build the docs when I unified the dependencies for local environments. I thought actually that we were already using those for building the docs in travis.

For me it makes much more sense than when we build the documentation locally, we use the same environment as when it's built in travis, I don't see why we would like to have different / less packages, or different versions. If we do what you suggest, upgrading python to 3.7 only for the docs, but not for local environemnts, or similar things, we'll find that we start working on the docs (let's say fixing the warnings), we get something working locally, and then when we upload the changes the docs can fail. I don't see the advantage.

If environment.yml is not synced anymore with the travis docs deps (or I made a mistake and they never were), I'd just add whatever is missing. Personally I don't want to have failing examples in my local docs built, for saving few resources on installing couple more dependencies.

But feel free to merge if you disagree, not a big deal.

@jorisvandenbossche
Copy link
Member Author

jorisvandenbossche commented Jun 3, 2019

I guess I'm missing something.

Well, just look at the log output, and you will see that there is something missing in the environment / some things not working in the doc build.
For azure I saw errors related to pyarrow and clipboard, those might just need to be added to environment.yml. On travis it was actually not even completing the build (erroring on a missing file). I am not really sure how that is related to the environment, but at least it seems to be fixed here, for some reason.

I am fine with unifying the environments, I won't object that (the environment.yml is already huge anyhow .. so that ship has sailed). But let's try to do that again in a proper way in a new PR. So going to merge this, just to have our doc build working again (we already introduced some new errors in recent PRs, which was not visible any more).

@jorisvandenbossche jorisvandenbossche merged commit 23b0788 into pandas-dev:master Jun 3, 2019
@jorisvandenbossche jorisvandenbossche deleted the fixup-travis-docs branch June 3, 2019 05:35
@datapythonista
Copy link
Member

Thanks for fixing those. I agree on what you say, and it was my bad for not detecting the failure myself. I was planning to check the new docs built, but azure-pipelines started to fail randomly, and the priority was to restore building master, and I didn't time to look much at the travis changes in the logs.

My point was that the docs should build with environment.yml, I think we agree on that, will have a look. And hopefully we can get rid of the warnings, and move the docs build to azure and fail the CI in case of errors, and we'll avoid all this kind of problems, which is not the first time we have. We'll have some people working on it this week, let's see if we succeed.

vaibhavhrt pushed a commit to vaibhavhrt/pandas that referenced this pull request Jun 6, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
CI Continuous Integration Docs
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants