Skip to content
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

Use conda-env for CI environments #20767

Merged
merged 41 commits into from
Apr 24, 2018
Merged

Conversation

TomAugspurger
Copy link
Contributor

@TomAugspurger TomAugspurger commented Apr 20, 2018

Closes #20720

The old setup where things were installed in multiple places resulted in some strange environments, with multiple copies of the same package in site-packages.

This attempts to reduce the environment definition to a single file per CI job, and the environment installation to 3 steps

  • create the environment
  • uninstall pandas if it's present (installing a pandas dependency)
  • build pandas from source & install

Travis only for now. Will move over the appveyor / circle ones too.

@TomAugspurger TomAugspurger added the CI Continuous Integration label Apr 20, 2018
That's pinned to numpy 1.10.4, which forces fastparquet 0.1.3, which
is not compatible with thrift & pandas master.
@codecov
Copy link

codecov bot commented Apr 20, 2018

Codecov Report

Merging #20767 into master will decrease coverage by 0.03%.
The diff coverage is n/a.

Impacted file tree graph

@@            Coverage Diff             @@
##           master   #20767      +/-   ##
==========================================
- Coverage   91.85%   91.81%   -0.04%     
==========================================
  Files         153      153              
  Lines       49308    49310       +2     
==========================================
- Hits        45290    45274      -16     
- Misses       4018     4036      +18
Flag Coverage Δ
#multiple 90.2% <ø> (-0.04%) ⬇️
#single 41.87% <ø> (-0.02%) ⬇️
Impacted Files Coverage Δ
pandas/plotting/_converter.py 65.07% <0%> (-1.74%) ⬇️
pandas/core/nanops.py 95.13% <0%> (-1.17%) ⬇️
pandas/core/indexes/base.py 96.68% <0%> (ø) ⬆️
pandas/core/frame.py 97.17% <0%> (ø) ⬆️
pandas/core/accessor.py 98.71% <0%> (+0.01%) ⬆️

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 0ae7e90...8b0e277. Read the comment docs.

@TomAugspurger
Copy link
Contributor Author

Just one failure, on the OSX build. That's pinned to numpy 1.10.4, which forces fastparquet 0.1.3, which is not compatible with thrift 0.11. I could pin fastparquet & thrift to those versions, but I think fastparquet is adequately tested elsewhere, so I've removed it.

@TomAugspurger TomAugspurger changed the title [WIP]: Use conda-env for CI environments Use conda-env for CI environments Apr 20, 2018
@TomAugspurger
Copy link
Contributor Author

TomAugspurger commented Apr 22, 2018 via email

@jreback
Copy link
Contributor

jreback commented Apr 22, 2018

conda-forge is only used for packages that are not found in the main
channel. The 3.5-osx env uses s3fs, moto, and feather which aren't in main.

I understand. my point is in the existing setup, we install from defaults ONLY for most packages. I am loath to give that up on at least 1 build. I would either go back to 2 steps on 1 or more builds, or remove these deps (e.g. you can install from pip instead these packages).

@jreback
Copy link
Contributor

jreback commented Apr 22, 2018

I think you can completely remove the JOB= env variable as well here (or maybe rename ENV_FILE to JOB)

@jreback
Copy link
Contributor

jreback commented Apr 22, 2018

also I don't think the issue number is correct here, #20720 ?

@TomAugspurger
Copy link
Contributor Author

you can install from pip instead these packages

I'd like to avoid installing moto from pip if possible. It brings in a lot of dependencies, which aren't managed by conda.

This closes #20720. It looks like that was caused by the 2 / 3 stage installs, since we ended up with multiple versions of requests, boto, etc., in the same environment. Now we just have the one. I haven't seen any of the network failures on this branch.

pip install cpplint
fi

if [ "$COVERAGE" ]; then
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

where did coverage go?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The coverage deps are run on the 3.6 build. travis-36.yaml has the additional coverage deps.

@jreback
Copy link
Contributor

jreback commented Apr 23, 2018

ok I recently added some xfails for #20720, can you remove in this PR?

@jorisvandenbossche
Copy link
Member

Does the conda-forge::packgage syntax work for environment files?
That might be another way to install some packages from conda-forge in an environment without adding conda-forge as a full channel (as then it might be more difficult to control which ones are installed from conda-forge / defaults)

@jorisvandenbossche
Copy link
Member

Small note: I personally liked the JOB tags to have a quick overview of the different build types. I know the ENV_FILE tag is similarly unique, but a bit less user friendly to read or to refer to ("check travis job ...")

@TomAugspurger
Copy link
Contributor Author

Does the conda-forge::packgage syntax work for environment files?

Not correctly unfortunately :( When I tried that, all the packages came from conda-forge. I suspect conda parses the deps for channels, and if found adds them to the start of the package list. Opened conda/conda#7202 for that.

This reverts commit f70a6dc.
@TomAugspurger
Copy link
Contributor Author

Reverted the JOB removal.

@jreback
Copy link
Contributor

jreback commented Apr 23, 2018

Small note: I personally liked the JOB tags to have a quick overview of the different build types. I know the ENV_FILE tag is similarly unique, but a bit less user friendly to read or to refer to ("check travis job ...")

I don't have a problem with the slightly longer ENV_FILE, maybe just rename this to JOB?
having the duplicated ones is a bit annoying

@jorisvandenbossche
Copy link
Member

Or maybe like before where the JOB tag is the variable part in the file name?

@TomAugspurger
Copy link
Contributor Author

TomAugspurger commented Apr 23, 2018 via email

@jorisvandenbossche
Copy link
Member

By file name, do you mean the environment.yaml? Because if so, I think that's the case.

Yes, that is what I meant. All environment files for travis are like ci/travis-specifics.yml, so JOB could just indicate 'specifics' (which would almost be identical to the current job tag, except for casing) to avoid duplication between JOB and ENV_FILE ?

@TomAugspurger
Copy link
Contributor Author

Re-pushed with more informative job tags. Now they include things like indicating which one is linting, running coverage, etc.

screen shot 2018-04-23 at 1 02 14 pm

Easier to tell at a glance which ones are special.

@jreback
Copy link
Contributor

jreback commented Apr 24, 2018

lgtm

merge away

@TomAugspurger TomAugspurger merged commit 41db527 into pandas-dev:master Apr 24, 2018
@TomAugspurger TomAugspurger deleted the ci-envs branch April 24, 2018 01:35
@TomAugspurger
Copy link
Contributor Author

FYI, the channel::package support in environment.yaml was recently added to conda in conda/conda#7178

I tried it out with conda-canary and things seemed to work fine.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
CI Continuous Integration
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants