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

Put conda-forge over defaults in windows tests #5228

Merged
merged 3 commits into from Aug 6, 2019

Conversation

jcrist
Copy link
Member

@jcrist jcrist commented Aug 5, 2019

Our windows tests are currently failing, likely due to a difference in
environment (numba 0.45.0 is being used from defaults in appveyor, numba
0.45.1 is being used from conda-forge on travis). We update our
environment.yml to specify using conda-forge over defaults when
possible.

Our windows tests are currently failing, likely due to a difference in
environment (numba 0.45.0 is being used from defaults in appveyor, numba
0.45.1 is being used from conda-forge on travis). We update our
environment.yml to specify using conda-forge over defaults when
possible.
@jcrist jcrist mentioned this pull request Aug 5, 2019
2 tasks
@jcrist
Copy link
Member Author

@jcrist jcrist commented Aug 5, 2019

Hmmm, this may also be due to pydata/sparse#257. Will wait for appveyor before looking further.

Copy link
Member

@jrbourbeau jrbourbeau left a comment

Thanks for handling this @jcrist! Left a couple of small comments

continuous_integration/appveyor/environment.yaml Outdated Show resolved Hide resolved
@@ -1,10 +1,11 @@
name: testenv
channels:
- conda-forge
- defaults
Copy link
Member

@jrbourbeau jrbourbeau Aug 5, 2019

Choose a reason for hiding this comment

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

Do we need the default channel, or does conda-forge still suffice?

Copy link
Member

@jakirkham jakirkham Aug 6, 2019

Choose a reason for hiding this comment

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

This looks correct to me. Dropping it would mean we don't use defaults at all, which is probably not what we want.

Copy link
Member

@jrbourbeau jrbourbeau Aug 6, 2019

Choose a reason for hiding this comment

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

Is there a reason to use defaults if all the packages we want to install are in conda-forge?

Copy link
Member Author

@jcrist jcrist Aug 6, 2019

Choose a reason for hiding this comment

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

Dropping it would mean we don't use defaults at all, which is probably not what we want.

Actually, IIUC this change was worthless except for explicitly orderining conda-forge and defaults - we'd have to add nodefaults to the channel list to disable defaults entirely (otherwise its implicitly there): https://docs.conda.io/projects/conda/en/latest/user-guide/tasks/manage-environments.html

The real fix here was to pin numpy, but I'm reluctant to make another commit since this PR works as is and this change is harmless.

Copy link
Member

@jrbourbeau jrbourbeau Aug 6, 2019

Choose a reason for hiding this comment

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

If so, we may consider adding defaults to our other CI environments (doesn't have to be in this PR)

Copy link
Member

@jrbourbeau jrbourbeau Aug 6, 2019

Choose a reason for hiding this comment

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

I'm reluctant to make another commit since this PR works as is and this change is harmless

Totally fine by me. I was just curious if I was missing something about how specifying channels works with conda

@jakirkham
Copy link
Member

@jakirkham jakirkham commented Aug 6, 2019

Some minor comments. Otherwise LGTM. 🙂

Co-Authored-By: jakirkham <jakirkham@gmail.com>
@jcrist jcrist merged commit e8741ae into dask:master Aug 6, 2019
2 checks passed
@jcrist jcrist deleted the use-conda-forge-by-default branch Aug 6, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants