Skip to content

A few fixes to bin scripts and a proposal of a new one #1596

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

Merged
merged 3 commits into from
Sep 7, 2016

Conversation

alexsavio
Copy link
Contributor

@alexsavio alexsavio commented Aug 31, 2016

  • fix nipype_crash_search to print usage on no args.
  • change nipype_crash_search shebang to #!python.
  • add nipype_display_pklz to look into .pklz node files.

@coveralls
Copy link

Coverage Status

Coverage decreased (-0.3%) to 71.99% when pulling 943fcad on alexsavio:master into da1f544 on nipy:master.

1 similar comment
@coveralls
Copy link

Coverage Status

Coverage decreased (-0.3%) to 71.99% when pulling 943fcad on alexsavio:master into da1f544 on nipy:master.

@coveralls
Copy link

coveralls commented Aug 31, 2016

Coverage Status

Coverage remained the same at 72.263% when pulling 943fcad on alexsavio:master into da1f544 on nipy:master.

@alexsavio alexsavio changed the title fix nipype_crash_search to print usage on no args A few fixes to bin scripts and a proposal of a new one Sep 1, 2016
@coveralls
Copy link

Coverage Status

Coverage remained the same at 72.263% when pulling e8fb466 on alexsavio:master into da1f544 on nipy:master.

1 similar comment
@coveralls
Copy link

coveralls commented Sep 1, 2016

Coverage Status

Coverage remained the same at 72.263% when pulling e8fb466 on alexsavio:master into da1f544 on nipy:master.

@coveralls
Copy link

Coverage Status

Coverage decreased (-0.03%) to 72.231% when pulling e8fb466 on alexsavio:master into da1f544 on nipy:master.

3 similar comments
@coveralls
Copy link

Coverage Status

Coverage decreased (-0.03%) to 72.231% when pulling e8fb466 on alexsavio:master into da1f544 on nipy:master.

@coveralls
Copy link

Coverage Status

Coverage decreased (-0.03%) to 72.231% when pulling e8fb466 on alexsavio:master into da1f544 on nipy:master.

@coveralls
Copy link

Coverage Status

Coverage decreased (-0.03%) to 72.231% when pulling e8fb466 on alexsavio:master into da1f544 on nipy:master.

@coveralls
Copy link

coveralls commented Sep 1, 2016

Coverage Status

Coverage remained the same at 72.263% when pulling e8fb466 on alexsavio:master into da1f544 on nipy:master.

@@ -1,11 +1,12 @@
#!/usr/bin/env python
#!python
Copy link
Member

Choose a reason for hiding this comment

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

could you please explain the rationale for this change?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

"#!python" is being used in the other scripts, e.g., nipype_display_crash.
I tought about it and I am not sure if "#!/usr/bin/env python" works on Windows (if there is any Windows user here).

@satra
Copy link
Member

satra commented Sep 7, 2016

@alexsavio - i'll merge this, but i'm noting here that one thing we should do is have a single nipype command tool with subcommands, like aws, singularity, docker, etc.,.

@satra satra merged commit 4eb197f into nipy:master Sep 7, 2016
@alexsavio
Copy link
Contributor Author

Hi Satra, thanks for the note.
I could do the subcommands for a nipype command. Would you have a dependency like click to make things easier or would you rather make something without dependencies?
I see something like:
nipype display_crash instead of nipype_display_crash,
nipype crash_search instead of nipype_crash_search,
nipype run instead of nipype_cmd, and
nipype convert instead of nipype2boutiques
?

@satra
Copy link
Member

satra commented Sep 8, 2016

let's go with click for now to start a refactor.

also perhaps nipype display and nipype search, which could be augmented later

@alexsavio
Copy link
Contributor Author

I have a first working approach. I moved nipype_display_crash to nipype crash, nipype_crash_search to nipype search, and nipype_show_pklz to nipype show.
nipype_cmd will be more involved since I have to process argv.
nipype2boutiques is straightforward, however I can't think on how to name it.

@satra
Copy link
Member

satra commented Sep 8, 2016

perhaps nipype convert --format=boutiques

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.

3 participants