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 packages instead of pip #989

Merged
merged 10 commits into from
Mar 8, 2019

Conversation

kosack
Copy link
Contributor

@kosack kosack commented Feb 27, 2019

No more pip installs in environment.yml

  • now use eventio (and corsikaio) packages on cta-observatory conda channel
  • also use pytest-runner from conda (was pip's pytest_runner)

@maxnoe
Copy link
Member

maxnoe commented Feb 27, 2019

Where do these images come from?
E.g. how are they build and uploaded?
How do we make sure, that we get conda images fast after each new release?

@maxnoe
Copy link
Member

maxnoe commented Feb 27, 2019

You also removed pyhessio from the py* yamls, which we should not do as it is needed for the tests.

@codecov
Copy link

codecov bot commented Feb 27, 2019

Codecov Report

Merging #989 into master will increase coverage by 0.58%.
The diff coverage is 100%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #989      +/-   ##
==========================================
+ Coverage   78.19%   78.77%   +0.58%     
==========================================
  Files         194      195       +1     
  Lines       11313    11328      +15     
==========================================
+ Hits         8846     8924      +78     
+ Misses       2467     2404      -63
Impacted Files Coverage Δ
ctapipe/io/lsteventsource.py 17.3% <ø> (+17.3%) ⬆️
ctapipe/io/simteleventsource.py 100% <ø> (ø) ⬆️
ctapipe/io/tests/test_available_sources.py 100% <100%> (ø)
ctapipe/io/__init__.py 100% <100%> (ø) ⬆️
ctapipe/io/nectarcameventsource.py 18.34% <0%> (+18.34%) ⬆️
ctapipe/io/sst1meventsource.py 21.62% <0%> (+21.62%) ⬆️

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 dcb64d4...ca412a0. Read the comment docs.

@kosack
Copy link
Contributor Author

kosack commented Feb 27, 2019

Where do these images come from?
E.g. how are they build and uploaded?
How do we make sure, that we get conda images fast after each new release?

I was just discussion that offline in fact... Right now I have to do it all manually, which is a pain. It would be nice to automate the builds. So far all the recipes are in https://github.com/cta-observatory/cta-conda-recipes. I manually edit them, and then run conda build on each (both on a macOS and SL6 VM), and then run anaconda upload --user cta-observaory to send them to send them to the repo.

For automation, I see the following options:

  • use conda-forge (may be overkill)
  • roll our own script to build them using travis (or the Jenkins instance that CTA has running)

To make them only build when necessary, it could be useful to split them into multiple repos (e.g. ctapipe-feedstock, eventio-feedstock) as done for conda-forge, so then we can have the builds of each package independently.

@kosack
Copy link
Contributor Author

kosack commented Feb 27, 2019

Ah, yes, I'll add pyhessio back

@maxnoe
Copy link
Member

maxnoe commented Feb 27, 2019

To make them only build when necessary, it could be useful to split them into multiple repos (e.g. ctapipe-feedstock, eventio-feedstock) as done for conda-forge, so then we can have the builds of each package independently.

Why own repos? Why not have the conda recipes in the corresponding repos and build on new tags?

@maxnoe
Copy link
Member

maxnoe commented Feb 27, 2019

@maxnoe
Copy link
Member

maxnoe commented Feb 27, 2019

We removed pyhessio from environment.yaml as this is what uses should install and had it only in the env*.yaml which are used on travis.

@kosack
Copy link
Contributor Author

kosack commented Feb 28, 2019

Why own repos? Why not have the conda recipes in the corresponding repos and build on new tags?

That's the recommended way, since you can update the recipe without updating the code (bumping the build number, but staying with the same code base). The recipe contains a link to the git repo and tag, so it doesn't need to be in the package itself. See e.g. conda-forge

@maxnoe
Copy link
Member

maxnoe commented Feb 28, 2019

Ok, but that automatically means, we'll always need human interaction to produce new conda builds for new versions, right? This is quite unfortunate.

@kosack
Copy link
Contributor Author

kosack commented Feb 28, 2019

Ok, but that automatically means, we'll always need human interaction to produce new conda builds for new versions, right? This is quite unfortunate.

How are the pip build uploaded? I assume also by human interaction? But yes, I think this is ok, and will be better once it's automated. We should assume "official" packages are used as dependencies.

@dneise
Copy link
Member

dneise commented Feb 28, 2019

How are the pip build uploaded?

For eventio it is done like this:
https://github.com/cta-observatory/pyeventio/blob/master/.travis.yml#L43

When a new tag is pushed to the master, the package is pushed to pypi, of course only when the tests pass. I.e. by using the "draft a new release" button on https://github.com/cta-observatory/pyeventio/releases the upload to pypi is triggered and nothing needs to be done in addition

@maxnoe
Copy link
Member

maxnoe commented Feb 28, 2019

Yes, but those are source distributions, not binary wheels, I don't think it's possible to build manylinux wheels on travis and I'm also not sure we want to go with many linux builds for performance reasons (ancient compilers, no modern instructions, no mkl)

@kosack
Copy link
Contributor Author

kosack commented Mar 4, 2019

Anyhow, I think we can move this conversion to the issue on packaging, and just accept this PR. The main issue is on the grid where we want everything installed in conda envs. It's not a big issue, the user can still install via pip if they like, but this keeps everything uniform.

@kosack kosack merged commit 13efd83 into cta-observatory:master Mar 8, 2019
@kosack kosack deleted the use_conda_for_eventio branch August 21, 2019 11:10
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