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

remove mamba reference #166

Closed
wants to merge 2 commits into from
Closed

remove mamba reference #166

wants to merge 2 commits into from

Conversation

jGaboardi
Copy link
Member

This PR removes the mamba reference in unittests.yml, which appears to be screwing up the channel priority settings resulting in a bad rtree install on Windows.

See pysal/spaghetti#610 and conda-forge/rtree-feedstock#31

@codecov-commenter
Copy link

codecov-commenter commented Apr 21, 2021

Codecov Report

Merging #166 (974624a) into master (2fbc439) will not change coverage.
The diff coverage is n/a.

Impacted file tree graph

@@           Coverage Diff           @@
##           master     #166   +/-   ##
=======================================
  Coverage   85.79%   85.79%           
=======================================
  Files          66       66           
  Lines        2788     2788           
=======================================
  Hits         2392     2392           
  Misses        396      396           

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 2fbc439...974624a. Read the comment docs.

@martinfleis
Copy link
Member

Does this essentially disable mamba? It would be great to figure out if there's an actual bug in mamba or some other way how to make it work (with mamba).

@jGaboardi
Copy link
Member Author

Does this essentially disable mamba? It would be great to figure out if there's an actual bug in mamba or some other way how to make it work (with mamba).

Yep, the only thing it does is remove the mamba-version: '*' keyword from conda-incubator/setup-miniconda. I think the most likely reason for the weird behavior is that we were using it incorrectly. Maybe we can get @goanpeca or @bollwyvl's opinion on this?

@jGaboardi
Copy link
Member Author

Maybe I am wrong. See here.

@jGaboardi
Copy link
Member Author

jGaboardi commented Apr 21, 2021

I now see that the values acceptable in conda for channel-priority have changed since we first started using it and we had not updated. Now channel-priority: true defaults to channel-priority: flexible. We should be using channel-priority: strict.

@knaaptime
Copy link
Member

thanks for this @jGaboardi. I'll keep an eye on what you're doing in spaghetti and try and incorporate those changes in the refactor branch also

@jGaboardi
Copy link
Member Author

thanks for this @jGaboardi. I'll keep an eye on what you're doing in spaghetti and try and incorporate those changes in the refactor branch also

@martinfleis and I had a major success with micromamba over the weekend that reduced total testing duration by more than half. Have a look and let me know if you'd like a PR for segregation.

@knaaptime
Copy link
Member

awesome. if you're up for a PR against my 2.0 branch, that would be killer

@knaaptime
Copy link
Member

(then we could close this one, actually)

@jGaboardi jGaboardi closed this Apr 27, 2021
AnGWar26 added a commit to AnGWar26/geosnap that referenced this pull request Apr 28, 2021
@jGaboardi jGaboardi deleted the jGaboardi-patch-1 branch June 18, 2021 19:36
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.

4 participants