Skip to content
This repository has been archived by the owner on May 18, 2021. It is now read-only.

Considering moving to Python #158

Open
jcohenadad opened this issue Jul 2, 2020 · 11 comments
Open

Considering moving to Python #158

jcohenadad opened this issue Jul 2, 2020 · 11 comments

Comments

@jcohenadad
Copy link
Member

jcohenadad commented Jul 2, 2020

Pros:

  • Auto-documentation is no more an issue with sphinx
  • Python is free
  • CI testing can be done on the cloud, e.g. with Travis (compared to our difficult-to-maintain local Azure server)
  • Active neuroimaging community (nibabel, dcm2bids, etc.)

Cons:

@gab-berest
Copy link

gab-berest commented Jul 2, 2020

Pros:
Community is a lot more active for Python than Matlab for every aspect, not just the neuroimaging community.
Really complete unit test libraries and dummy libraries in Python.

@kousu
Copy link
Contributor

kousu commented Jul 3, 2020

https://numpy.org/devdocs/user/numpy-for-matlab-users.html helps move from matlab to python for scientific computation.

@kousu
Copy link
Contributor

kousu commented Jul 3, 2020

@mathieuboudreau
Copy link
Member

mathieuboudreau commented Jul 3, 2020

It really isn't that difficult of a transition (MATLAB->Python, not rewriting code) in my opinion - you just need to know/find which tools have the functions that you're used to using in MATLAB. Also know that everything in Python is a class object "underneath the hood". The biggest hurdle is if any of your features depend on a tool that was written for MATLAB - rewriting that tool is likely out of the question, so you either have to find a replacement or give up on that feature.

And once you switch from MATLAB to Python, the added features that @jcohenadad listed and many more (e.g. a very active open-source community, jupyter notebooks, binder, plotly, etc.) will make your life so much easier that you might never want to turn back...

one-of-us-1-one-of-us-34138353

@jcohenadad
Copy link
Member Author

Strategy is to:

  • start a new repos: shimming-toolbox-py
  • implement some scripts that will execute basic (but highly-needed) functionalities:
    • dcm2nii2bids
    • unwrap phase
    • create b0 maps
  • setup: travis CI, sphinx doc, unit testing (pytest)
  • in parallel, some of the refactoring can continue in the matlab repos, in order to facilitate the porting to python later on
  • new features (e.g. numerical simu) could continue in matlab for now
  • mid/long term: once we find that the python repos has enough functionalities, we could do the switch and archive the matlab repos.

@jcohenadad
Copy link
Member Author

@kousu
Copy link
Contributor

kousu commented Jul 3, 2020

For an example, to port the b0 map step, I would start by making this CLI:

./b0maps [--coilshims arduino_config.txt] [--pulseseq pulse_calibration.txt] dicom_folder/ output_fieldmap.nii

For some starter code, you can use argparse, but I am a real big fan of click, so that's the sketch I'm going to write now:

#!/usr/bin/env python3

import click

import logging

@click.command()
@click.option("--verbose", is_flag=True, help="Be more verbose.")
@click.option("--coilshims", help="Output hardware-shim calibration.")
@click.option("--pulseseq", help="Output pulse-sequence calibration.")
@click.argument("input")
@click.argument("output")
def cli(verbose, coilshims, pulseseq, input, output):
    """
    Compute b0 fieldmap OUTPUT from a folder of dicom files INPUT.
    """
    if verbose:
        logging.getLogger().setLevel(logging.INFO)
    logging.info(f'{coilshims}, {pulseseq}, {input}, {output}')
    raise NotImplementedError()

if __name__ == '__main__':
    cli()

On mac/linux you can put that into "b0maps" and then run it like:

$ chmod +x b0maps
$ ./b0maps --verbose dcms map.nii.gz
INFO:root:None, None, dcms, map.nii.gz
Traceback (most recent call last):
  File "./b0maps", line 23, in <module>
    cli()
  File "/home/kousu/.local/lib/python3.6/site-packages/click/core.py", line 829, in __call__
    return self.main(*args, **kwargs)
  File "/home/kousu/.local/lib/python3.6/site-packages/click/core.py", line 782, in main
    rv = self.invoke(ctx)
  File "/home/kousu/.local/lib/python3.6/site-packages/click/core.py", line 1066, in invoke
    return ctx.invoke(self.callback, **ctx.params)
  File "/home/kousu/.local/lib/python3.6/site-packages/click/core.py", line 610, in invoke
    return callback(*args, **kwargs)
  File "./b0maps", line 20, in cli
    raise NotImplementedError()
NotImplementedError

On Windows the first line, the "shebang", doesn't work, but you can save it to b0maps.py instead and do

C:\...\> python b0maps.py --verbose dcms map.nii.gz

@jcohenadad
Copy link
Member Author

thank you for the head start here shimming-toolbox/shimming-toolbox#158 (comment) @kousu.

however, i would suggest to separate CLIs as much as possible, i.e. not mix dcm2nii conversion and b0map fitting (because we know that later one these will have to be separated anyway)

how about following the latest examples we did? @gaspardcereza @po09i are working on example scenario scripts, so maybe we can follow this sequence of calls instead.
also, currently, those example scripts are in gdoc, which is not visible for everyone, and is also confusing for newcomers to this project. i would suggest to:

  • start an example file under shimming-toolbox-py/examples
    • this example file could be the latest version of the gdoc you are working on @gaspardcereza @po09i
    • once you have moved that in the GH repository, please, at the beginning of the gdoc, write something like "PLEASE DO NOT USE THIS DOCUMENT ANYMORE. INSTEAD, GO TO: ". This is good management practice.
  • this example file would list the different sequential commands: convert dcm2nii, organize to bids, unwrap, fit b0, etc.
  • for each command, listed in the example file, create the corresponding cli

does that make sense?

@gaspardcereza
Copy link
Member

Right now we don't have all the functions required to run a full scenario (from dicoms to shim calculation) so that's why we decided to go with a gdoc instead of a code that wouldn't be fully functional. Would it be ok if the parts of the code that are not available yet were just commented until we create them ?
That would also allow us to easily keep track of what has been done so far and what remains to be done.

@jcohenadad
Copy link
Member Author

Right now we don't have all the functions required to run a full scenario (from dicoms to shim calculation) so that's why we decided to go with a gdoc instead of a code that wouldn't be fully functional. Would it be ok if the parts of the code that are not available yet were just commented until we create them ?

definitely ok! the goal here is to get started immediately with something

That would also allow us to easily keep track of what has been done so far and what remains to be done.

yup!

again, i would strongly encourage to centralize everything in github. This is much easier to manage the project.

@jcohenadad
Copy link
Member Author

this is happening 🙌 --> https://github.com/shimming-toolbox/shimming-toolbox-py

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

5 participants