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

Conda #112

Draft
wants to merge 67 commits into
base: master
Choose a base branch
from
Draft

Conda #112

wants to merge 67 commits into from

Conversation

paskino
Copy link
Contributor

@paskino paskino commented Apr 26, 2018

initial working recipe for linux.

OSX would need tweak on build.sh.

The branch should be aligned with setup_py #109

@casperdcl
Copy link
Member

  1. merge Fix python dest SIRF#160
  2. merge expose python strategy #109
  3. rebase this and merge

close #106?

@paskino
Copy link
Contributor Author

paskino commented Apr 26, 2018

I agree with you.

currently the boost in conda-forge does not contain all the libs we need. So we need to build and install.
@casperdcl casperdcl mentioned this pull request Apr 26, 2018
@KrisThielemans
Copy link
Member

should we add a conda build to Travis? Maybe it's already done via the docker build.

that'd allow you to test on OSX.

@casperdcl
Copy link
Member

casperdcl commented Apr 27, 2018

EDIT new strategy:

misc:

  • add conda build to travis (I think getting on conda-forge would avoid this)
  • close Fix python dest #106

@paskino
Copy link
Contributor Author

paskino commented Jul 9, 2019

Directories to pass to External_SIRF.cmake are defined

Copy link
Member

@KrisThielemans KrisThielemans left a comment

Choose a reason for hiding this comment

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

I haven't checked this really, but I'm confused about the various build.sh files, and then sirf/build.sh building other stuff anyway.

@@ -0,0 +1,72 @@
#!/bin/bash
Copy link
Member

Choose a reason for hiding this comment

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

I would try to avoid building boost at all costs. Use the conda dependency

@@ -0,0 +1,79 @@
#!/bin/bash
Copy link
Member

Choose a reason for hiding this comment

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

let's not build hdf5

-DPYTHON_DEST_DIR=${PREFIX}/python\
-USIRF_URL \
-USIRF_TAG \
-DSIRF_TAG=v2.1.0\
Copy link
Member

Choose a reason for hiding this comment

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

let's not put explicit versions in any of the conda build.sh. In any case, SIRF version is irrelevant in ismrmrd/build.sh

Comment on lines 70 to 72
make -j2 ITK
make -j2 NIFTYREG
make -j2 STIR
Copy link
Member

Choose a reason for hiding this comment

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

no need for these.

-DUSE_SYSTEM_GTest=On\
-DCONDA_BUILD=On

make -j1 Gadgetron
Copy link
Member

Choose a reason for hiding this comment

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

in stir/build.sh?

@paskino paskino marked this pull request as draft April 26, 2020 20:11
@KrisThielemans
Copy link
Member

STIR 4.0.1 conda might work ok. There's https://anaconda.org/inati/ismrmrd but it's out-of-date. Might be worth checking with them.

@KrisThielemans
Copy link
Member

By the way, I'm not 100% sure what the strategy here was/is, but I think we should have a SIRF-only conda package (with its dependencies, most obtained via conda). I would at least initially not attempt gadgetron-via-conda.

@KrisThielemans
Copy link
Member

See gadgetron/gadgetron#1036 for Gadgetron's conda efforts

- vc 14 # [win and py35]
- vc 9 # [win and py27]
- python 3.6 # [py36]
- python 2.7 # [py27]
Copy link
Member

Choose a reason for hiding this comment

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

drop this of course

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.

5 participants