populse / capsul

Collaborative Analysis Platform : Simple, Unifying, Lean
Other
7 stars 14 forks source link

add mrtrix in capsul #288

Closed manuegrx closed 11 months ago

manuegrx commented 11 months ago

Hi,

I would like to add mrtrix (mrtrix3) in Capsul in order to use it in populse mia.

As I am not sure about which is the new method and which is the old one, I modified study_config, in_context and engine methods.

It seems to work fine on my side when I used mrtrix in populse (see https://github.com/populse/populse_mia/pull/313) and when I used the script the following script (with a config file: python -m capsul --config=/tmp/config_mrtrix.json /tmp/mrtrix_capsul.py /tpm/out_config.json):

from capsul.api import Process
from traits.api import File
from capsul.in_context import mrtrix
import json

class mrinfo_help(Process):
    output_config = File(output=True, desc='output file to write config',
                         allowed_extensions=['.json'])

    def requirements(self):
        return {'mrtrix': 'any'}

    def _run_process(self):
        import capsul.engine
        fconf = capsul.engine.configurations.get(
            'capsul.engine.module.mrtrix')
        if not fconf:
            raise RuntimeError('mrtrix config is not present')
        with open(self.output_config, 'w') as f:
            json.dump(fconf, f)
        mrtrix.mrtrix_check_call(['mrinfo', '-h'])

Are there any other tests I need to perform?

Moreover, regarding the test done in Capsul I have few questions:

As it is my first steps in Capsul I think it could be a greta idea if someone can check what I have done :)

manuegrx commented 11 months ago

Thanks for your feedback !

I will wait for Denis's advice about the cif value before to merge this branch (there's no hurry about this subject)

manuegrx commented 11 months ago

Moreover some of the GitHub automatic test are failing, is it normal ?

denisri commented 11 months ago

Thanks for your feedback !

I will wait for Denis's advice about the cif value before to merge this branch (there's no hurry about this subject)

I haven't read the code thoroughly but it seems OK for me.

denisri commented 11 months ago

Moreover some of the GitHub automatic test are failing, is it normal ?

codespell has many false positive that I don't know how to disable. I'm not sure if we should use it at all, it's sometimes useful but makes checks systematically fail in many projects. For Macos tests, I have no idea...