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

Comprehensive CI. #178

Merged
merged 22 commits into from Jul 22, 2019
Merged

Comprehensive CI. #178

merged 22 commits into from Jul 22, 2019

Conversation

@hameerabbasi
Copy link
Contributor

hameerabbasi commented Jul 11, 2019

No description provided.

@hameerabbasi hameerabbasi force-pushed the comprehensive-ci branch 3 times, most recently from f4701a1 to ae36a87 Jul 11, 2019
@hameerabbasi

This comment has been minimized.

Copy link
Contributor Author

hameerabbasi commented Jul 11, 2019

@hameerabbasi hameerabbasi force-pushed the comprehensive-ci branch 4 times, most recently from 3134706 to 46758dc Jul 11, 2019
@rgommers

This comment has been minimized.

Copy link
Member

rgommers commented Jul 11, 2019

The macOS one looks straightforward; the combination of Dask 1.2 and Python 3.5 is not supported. So simply keep 3.5 testing for Linux, and bump macOS to 3.7

The Windows one is related to default conda installs not being on any path that Windows recognizes. I can think of various fixes, but the easiest is probably to find an existing CI that uses conda on Windows (e.g. https://github.com/geopandas/geopandas/blob/master/appveyor.yml#L30)

@larsoner

This comment has been minimized.

Copy link

larsoner commented Jul 11, 2019

To make your life easy with conda I'd use ci-helpers, it cuts down on a lot of the boilerplate code and is well maintained (has been for a few years now):

https://github.com/astropy/ci-helpers
https://github.com/mne-tools/mne-python/blob/maint/0.18/azure-pipelines.yml#L59-L74
https://github.com/mne-tools/mne-python/blob/maint/0.18/.travis.yml#L64-L65

Easily controllable using env vars:

https://github.com/mne-tools/mne-python/blob/maint/0.18/.travis.yml#L20-L26

On Anaconda, don't try to use an environment.yml with Windows + Python >= 3.7 though (has been broken for 6 mo with no resolution in sight):

conda/conda#8187

@larsoner

This comment has been minimized.

Copy link

larsoner commented Jul 11, 2019

... and beware following appveyor.yml code to construct your azure-pipelines.yml, you need to do some magical stuff at the end of commands in Azure to get env vars to persist that are not required by appveyor .

displayName: Create environment.

- script: |
source activate uarray

This comment has been minimized.

Copy link
@dhirschfeld

dhirschfeld Jul 12, 2019

With recent conda this should be:

Suggested change
source activate uarray
conda activate uarray

On Windows I'd recommend using the latest 4.7.6 as there have been various issues with earlier versions in the 4.7* series.

steps:
- script: |
echo "##vso[task.prependpath]$CONDA/bin"
displayName: Prepare conda

This comment has been minimized.

Copy link
@dhirschfeld

dhirschfeld Jul 12, 2019

It doesn't look like this task did anything?

image

...though maybe you only get output if you run with the debug variable set

This comment has been minimized.

Copy link
@dhirschfeld

dhirschfeld Jul 12, 2019

I think perhaps it's the $CONDA which might not work as it's not valid cmd.exe syntax? It might be worth trying the pipelines syntax $(CONDA)/bin or cmd.exe syntax %CONDA%/bin

@hameerabbasi hameerabbasi force-pushed the comprehensive-ci branch 2 times, most recently from d4da858 to e25c0e6 Jul 12, 2019
@hameerabbasi hameerabbasi force-pushed the comprehensive-ci branch from e25c0e6 to a23b573 Jul 12, 2019
@hameerabbasi hameerabbasi force-pushed the comprehensive-ci branch from 6a80296 to ab467ef Jul 12, 2019
@hameerabbasi hameerabbasi force-pushed the comprehensive-ci branch from ab467ef to 2e29b69 Jul 12, 2019
@hameerabbasi

This comment has been minimized.

Copy link
Contributor Author

hameerabbasi commented Jul 14, 2019

Do the current failures make sense? 😕 (except the docs one)

@peterbell10

This comment has been minimized.

Copy link
Collaborator

peterbell10 commented Jul 14, 2019

I think black might be the issue, from its readme:

Black can be installed by running pip install black. It requires Python 3.6.0+ to run

You should only need to check code formatting on one platform anyway.

@hameerabbasi

This comment has been minimized.

Copy link
Contributor Author

hameerabbasi commented Jul 15, 2019

@peterbell10 According to the error message, it seems pip only works on Python 3.6+. I really do think it's a solver error.

- mypy
- pytorch-cpu
- doc8
- black

This comment has been minimized.

Copy link
@peterbell10

peterbell10 Jul 15, 2019

Collaborator

You're still installing black on python35

@peterbell10

This comment has been minimized.

Copy link
Collaborator

peterbell10 commented Jul 15, 2019

Try pip==18.0? Looks like that was the last version that had files with py35 in its name on conda-forge.

https://anaconda.org/conda-forge/pip/files?version=18.0

@peterbell10

This comment has been minimized.

Copy link
Collaborator

peterbell10 commented Jul 15, 2019

This really doesn't make sense now.

UnsatisfiableError: The following specifications were found to be incompatible with each other:

  • pip=18.0 -> python[version='>=3.5,<3.6.0a0']
@peterbell10

This comment has been minimized.

Copy link
Collaborator

peterbell10 commented Jul 15, 2019

If you remove pytest-mypy as well, you shouldn't have to install pip right?

@hameerabbasi hameerabbasi force-pushed the comprehensive-ci branch from 460f09a to 608762c Jul 15, 2019
@hameerabbasi

This comment has been minimized.

Copy link
Contributor Author

hameerabbasi commented Jul 15, 2019

This is weird, type hints existed in Py35.

conda config --set always_yes True
conda update -n base conda
conda init
conda env update -n uarray -f .conda/environment$(python_version).yml

This comment has been minimized.

Copy link
@larsoner

larsoner Jul 15, 2019

FYI Windows+3.7 can fail because of this bug:

conda/conda#8187

But if your pip-installed deps don't require numpy you might be okay

This comment has been minimized.

Copy link
@hameerabbasi

hameerabbasi Jul 15, 2019

Author Contributor

They don't.

This comment has been minimized.

Copy link
@dhirschfeld

dhirschfeld Jul 15, 2019

I wouldn't depend on pip at all when using conda - you can avoid a whole class of bugs and bad interaction by simply submitting PRs to staged-recipes to get all your deps available with conda

This comment has been minimized.

Copy link
@hameerabbasi

hameerabbasi Jul 16, 2019

Author Contributor

@dhirschfeld The 3.5 deps don't include pip and it still fails... Because it doesn't recognize type hints.

@hameerabbasi hameerabbasi force-pushed the comprehensive-ci branch from 4062b64 to 8dab245 Jul 16, 2019
@hameerabbasi hameerabbasi force-pushed the comprehensive-ci branch from 803f161 to f9a5d32 Jul 16, 2019
@hameerabbasi hameerabbasi force-pushed the comprehensive-ci branch from 50c20d9 to 109cc7f Jul 16, 2019
@hameerabbasi hameerabbasi force-pushed the comprehensive-ci branch from 109cc7f to b55ce90 Jul 16, 2019
@hameerabbasi

This comment has been minimized.

Copy link
Contributor Author

hameerabbasi commented Jul 22, 2019

The Windows failure is unrelated. Merging.

@hameerabbasi hameerabbasi merged commit 730aea6 into master Jul 22, 2019
1 of 2 checks passed
1 of 2 checks passed
Quansight-Labs.uarray #20190722.6 failed
Details
License Compliance All checks passed.
Details
@peterbell10 peterbell10 deleted the comprehensive-ci branch Jul 22, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
5 participants
You can’t perform that action at this time.