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

new traits api support #135

Open
wants to merge 9 commits into
base: master
Choose a base branch
from
Open

new traits api support #135

wants to merge 9 commits into from

Conversation

NicolasRannou
Copy link

Hi all,

I work with Ellen Grant, Rudolph Pienaar and Daniel Haehn at the FNNDSC , Boston Children's Hospital.
We modified the code a bit to support the new traits api.

Would it make sense to integrate it in the LTS5:master?

Thanks,
Nicolas

@agriffa
Copy link

agriffa commented Apr 25, 2012

Dear Nicolas,

Thank you very much for your contribution and help.
We looked at the modifications you did. I was only wondering: is the new
code completely back-compatible?
In this case, please go ahead with the merge into the LTS5 master.

Our purpose in developing the CMTK was encouraging external
contributions and offer a shared platform for further development.
So, thanks again! Are you planning to keep contributing to the code in
the future? In this case we would like to list you and your group on the
cmtk web page.

Kind regards,
Alessandra

On 04/23/2012 07:42 PM, Nicolas Rannou wrote:

Hi all,

I work with Ellen Grant, Rudolph Pienaar and Daniel Haehn at the FNNDSC , Boston Children's Hospital.
We modified the code a bit to support the new traits api.

Would it make sense to integrate it in the LTS5:master?

Thanks,
Nicolas

You can merge this Pull Request by running:

git pull https://github.com/NicolasRannou/cmp master

Or you can view, comment on it, or merge it online at:

#135

-- Commit Summary --

  • WIP: modifying code to fit the new enthought libraries
  • WIP: ubuntu12.04
  • Revert "WIP: ubuntu12.04"
  • modify the binaries for ubuntu 12.04 and modify the config file to handle both traits and enthough apis
  • looking for any boost library (no specific version attached)
  • remove prints
  • remove prints
  • add support new traits api
  • remove line break

-- File Changes --

M cmp/binary/linux2/bit64/DTB_P0 (0)
M cmp/binary/linux2/bit64/DTB_dtk2dir (0)
M cmp/binary/linux2/bit64/DTB_gfa (0)
M cmp/binary/linux2/bit64/DTB_streamline (0)
M cmp/configuration.py (6)
M cmp/connectome.py (1)
M cmp/gui.py (52)
M cmp/helpgui.py (20)
M cmp/pipeline/pipeline_pb2.py (99)
M cmp/stages/converter/dicomconverter.py (3)
M cmp/util.py (7)
M src/DTB/CMakeLists.txt (2)

-- Patch Links --

https://github.com/LTS5/cmp/pull/135.patch
https://github.com/LTS5/cmp/pull/135.diff


Reply to this email directly or view it on GitHub:
#135


Alessandra Griffa
Doctoral assistant

Signal Processing Laboratory 5
EPFL STI IEL LTS5
ELD 224 (Bâtiment ELD)
Station 11 CH-1015
Lausanne

Tel.: +41216935672

@NicolasRannou
Copy link
Author

Hi Alessandra,

It is working properly with the new traits api and shouldn't break any backward compatibility.
But you are right, it is more safe to test before merging.
It don't think I have any write access on LTS5/cmp anyway :)

I noticed that in my pull request, cmp/pipeline/pipeline_pb2.py is modified whereas I don't remember introducing those changes...

Therefore I'd suggest to hold on merging until:
-backward compatibility is tested
-figure out what happened in cmp/pipeline/pipeline_pb2.py (outdated branch?)

We definitely want to contribute more code, would this pull request be the best practice?
Let me know if you want to know more about our lab! :)
http://childrenshospital.org/cfapps/research/data_admin/Site3068/mainpageS3068P0.html
Some people of our group already contributed to CMP (see the readme)
Ellen Grant (director)
Daniel Ginsburg (danginsburg) (previous software developper)
Rudolph Pienaar (rudolphpienaar) (technical leader)

but I agree it would be great if we are also listed on the cmtk webpage!

Best,
Nicolas

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.

2 participants