[CCPPETMR/SIRF-SuperBuild] sirf python packaging (#109)

[Casper da Costa-Luis]

• provides setup.py
  • provides sirf
  • alias p(Gadgetron|STIR|Utilities) to sirf.p(Gadgetron|STIR|Utilities) for backward-compatibility
• cmake -> pip install -> setup.py
• depends on CCPPETMR/SIRF#160 (or CCPPETMR/SIRF#161)

old

• INSTALL/
  • python/
    • *.(py|so)

new

• INSTALL/
  • python/
    • setup.py
    • sirf/
      • __init__.py
      • *.(py|so)
    • p*/__init__.py

---

You can view, comment on, or merge this pull request online at:

  https://github.com/CCPPETMR/SIRF-SuperBuild/pull/109

Commit Summary

• multi-stage Dockerfile
• more staging, abstraction
• drop sudo, re-add debug output
• docker use openmp
• initial setup.py
• backward-compatible setup.py

File Changes

• M .travis.yml (11)
• M SuperBuild.cmake (55)
• M SuperBuild/External_SIRF.cmake (4)
• A SuperBuild/__init__.py.in (1)
• A SuperBuild/setup.py.in (39)
• M docker/Dockerfile (57)
• D docker/Dockerfile-core (54)
• A docker/devel/requirements-test.txt (2)
• M docker/devel/test.sh (22)
• M docker/docker-compose.devel.yml (2)
• M docker/docker-compose.nix.yml (2)
• M docker/docker-compose.yml (5)
• A docker/raw-ubuntu.sh (16)
• R docker/requirements-travis.txt (2)

Patch Links:

• https://github.com/CCPPETMR/SIRF-SuperBuild/pull/109.patch
• https://github.com/CCPPETMR/SIRF-SuperBuild/pull/109.diff

—
You are receiving this because you are subscribed to this thread.
Reply to this email directly, view it on GitHub, or mute the thread.

[Kris Thielemans]

hi. I didn't check this yet, but quite a few extra changes here related to Docker etc. intentional?

[Casper da Costa-Luis]

yes, it's some fairly no-brainer docker tidy up, would probably merge that in separately first (#110)

[Casper da Costa-Luis]

@paskino please note my comment in the description about using setup.py to call cmake - in which case we might be able to get away with

1. install miniconda
2. install requirements using apt-get (possibly almost nothing since most things might be on conda?)
3. conda install -c conda-forge sirf # will auto-install dependencies, then run setup.py on sirf-superbuild which will run cmake and make

[Edoardo Pasca]

Casper is setup.py already calling cmake to build sirf?

[Casper da Costa-Luis]

No, but I could make it that way. Right now the procedure is:

1. apt-get requirements (cmake, boost, etc)
2. create python virtualenv and install python requirements
3. cmake && make (will build everything as before, except for the following changes:)
   • PYTHONPATH is no longer altered anywhere
   • setup.py is generated and used in a pip install command to automatically pick up built python modules

[Edoardo Pasca]

Maybe that's not necessary then. conda recipe currently covers 1 and 2. If we split 3 in:

3.a cmake && make
3.b python setup.py install

I guess I could just merge this branch in the conda branch adding 3.b

[Kris Thielemans]

I don't see the benefit of splitting into 3a and 3b. "make" already calls setup.py. why split it?

I also don't think letting setup.py do cmake&&make is a good idea. It means you cannot build MATLAB and Python in one go.

(obviously, in the conda script you wouldn't want to call make but cmake --build -C Release or whatever the syntax is).

[Casper da Costa-Luis]

@casperdcl pushed 1 commit.

• 32b1332 missing packages

—
You are receiving this because you are subscribed to this thread.

View it on GitHub or mute the thread.

[Casper da Costa-Luis]

@casperdcl pushed 2 commits.

• 011cc36 copies the .so into the site-packages
• 1f74917 pip -> setup.py

—
You are receiving this because you are subscribed to this thread.

View it on GitHub or mute the thread.

[Edoardo Pasca]

The conda built SIRF works, except #111

[Casper da Costa-Luis]

btw, docker image sizes for different python sources:

• virtualenv: 2.84GB
• miniconda: 2.98GB
• miniconda (update --all): 3GB

[Casper da Costa-Luis]

@casperdcl pushed 2 commits.

• 08a9712 missing docker devel cache
• 9c32624 pyvenv -> miniconda

—
You are receiving this because you are subscribed to this thread.

View it on GitHub or mute the thread.

[Kris Thielemans]

@KrisThielemans commented on this pull request.

I feel that the SIRF specific stuff (i.e. 90%) should be in the SIRF repo itself, but not sure if that'd lead to a ton of duplication.
If we need to keep it here, it definitely needs to be in a separate file, presumably called from External_SIRF.cmake

---

In SuperBuild/setup.py.in:

> @@ -0,0 +1,40 @@
+#!/usr/bin/env python

is this ok? shouldn't it use PYTHONINTERP?

[Casper da Costa-Luis]

@casperdcl commented on this pull request.

---

In SuperBuild/setup.py.in:

> @@ -0,0 +1,40 @@
+#!/usr/bin/env python

should be fine, mainly just for correct syntax highlighting despite the .in extension - it's never executed directly (always ${PYTHON_EXECUTABLE} setup.py, not ./setup.py) so is ignored anyway

[Casper da Costa-Luis]

@casperdcl pushed 1 commit.

• 700d8cb add PYTHON_STRATEGY=PYTHONPATH, tidy before migration to SIRF

—
You are receiving this because you are subscribed to this thread.

View it on GitHub or mute the thread.

[Casper da Costa-Luis]

@casperdcl pushed 1 commit.

• eeecb47 move to SIRF/setup_py

—
You are receiving this because you are subscribed to this thread.

View it on GitHub or mute the thread.

[Casper da Costa-Luis]

Merged #109.