WIP: Move nipype commands to a group command #1608

alexsavio · 2016-09-08T17:22:08Z

This is a continuation of #1596.
I am using click to create a group command, instead of adding all commands as separate scripts in the bin folder.

coveralls · 2016-09-08T17:51:39Z

Coverage decreased (-0.09%) to 72.176% when pulling 4237c65 on alexsavio:master into 0f9a620 on nipy:master.

coveralls · 2016-09-08T17:51:39Z

Coverage decreased (-0.09%) to 72.176% when pulling 4237c65 on alexsavio:master into 0f9a620 on nipy:master.

coveralls · 2016-09-08T17:51:39Z

Coverage decreased (-0.09%) to 72.176% when pulling 4237c65 on alexsavio:master into 0f9a620 on nipy:master.

coveralls · 2016-09-08T17:51:39Z

Coverage decreased (-0.09%) to 72.176% when pulling 4237c65 on alexsavio:master into 0f9a620 on nipy:master.

coveralls · 2016-09-08T17:51:39Z

Coverage decreased (-0.09%) to 72.176% when pulling 4237c65 on alexsavio:master into 0f9a620 on nipy:master.

coveralls · 2016-09-08T17:51:39Z

Coverage decreased (-0.09%) to 72.176% when pulling 4237c65 on alexsavio:master into 0f9a620 on nipy:master.

coveralls · 2016-09-08T17:51:39Z

Coverage decreased (-0.09%) to 72.176% when pulling 4237c65 on alexsavio:master into 0f9a620 on nipy:master.

coveralls · 2016-09-09T15:27:30Z

Coverage decreased (-0.1%) to 72.125% when pulling b2c9867 on alexsavio:master into 0f9a620 on nipy:master.

coveralls · 2016-09-09T15:27:30Z

Coverage decreased (-0.1%) to 72.125% when pulling b2c9867 on alexsavio:master into 0f9a620 on nipy:master.

coveralls · 2016-09-09T15:27:30Z

Coverage decreased (-0.1%) to 72.125% when pulling b2c9867 on alexsavio:master into 0f9a620 on nipy:master.

coveralls · 2016-09-09T15:27:30Z

Coverage decreased (-0.1%) to 72.125% when pulling b2c9867 on alexsavio:master into 0f9a620 on nipy:master.

coveralls · 2016-09-09T15:43:12Z

Coverage decreased (-0.1%) to 72.141% when pulling b2c9867 on alexsavio:master into 0f9a620 on nipy:master.

coveralls · 2016-09-09T15:43:12Z

Coverage decreased (-0.1%) to 72.141% when pulling b2c9867 on alexsavio:master into 0f9a620 on nipy:master.

coveralls · 2016-09-09T15:43:12Z

Coverage decreased (-0.1%) to 72.141% when pulling b2c9867 on alexsavio:master into 0f9a620 on nipy:master.

coveralls · 2016-09-09T15:43:12Z

Coverage decreased (-0.1%) to 72.141% when pulling b2c9867 on alexsavio:master into 0f9a620 on nipy:master.

coveralls · 2016-09-09T15:43:12Z

Coverage decreased (-0.1%) to 72.141% when pulling b2c9867 on alexsavio:master into 0f9a620 on nipy:master.

alexsavio · 2016-09-09T16:51:18Z

I still can't figure out if there is any clean way of using Click for the --format option in the convert command. For now I added a to_boutiques command.

coveralls · 2016-09-09T18:05:45Z

Coverage decreased (-0.1%) to 72.124% when pulling 8207421 on alexsavio:master into 0f9a620 on nipy:master.

coveralls · 2016-09-09T18:05:45Z

Coverage decreased (-0.1%) to 72.124% when pulling 8207421 on alexsavio:master into 0f9a620 on nipy:master.

alexsavio · 2016-09-12T09:47:39Z

The only option I can see is to have a simple click command with a --format parameter which forwards the rest of arguments to the nipype2boutiques function that uses an ArgumentParser to parse the rest of arguments. What do you think?

satra · 2016-09-12T13:17:21Z

@alexsavio - this section (http://click.pocoo.org/5/quickstart/#nesting-commands) and the next suggests how to include subcommands and options.

so perhaps two sets of groups (one for main nipype_cli - e.g., show, convert) and one for format type (boutique, cwl, etc.,.) under convert with their own options would work.

so something along the lines of this example (without the chaining):

pallets/click#415

alexsavio · 2016-09-12T13:43:58Z

Thanks @satra. Now it looks like nipype convert boutiques and the --help option works as expected as well.
I tested all the commands except the convert boutiques one. It would be nice to have an example of how to run it.

codecov-io · 2016-09-12T13:56:35Z

Current coverage is 70.82% (diff: 29.91%)

Merging #1608 into master will increase coverage by 1.01%

@@             master      #1608   diff @@
==========================================
  Files           995       1032     +37   
  Lines         48658      51798   +3140   
  Methods           0          0           
  Messages          0          0           
  Branches       7317       7341     +24   
==========================================
+ Hits          33965      36685   +2720   
- Misses        13271      14024    +753   
+ Partials       1422       1089    -333

Powered by Codecov. Last update 43ac390...3e650d4

alexsavio · 2016-09-19T11:59:43Z

Hi,
I think this is ready. I took the liberty of removing the old bin implementation, also from the setup file.
Please have a look.

satra · 2016-10-01T20:29:23Z

@alexsavio - could we merge with current master and make sure the tests pass for this.

alexsavio · 2016-10-04T16:21:15Z

yes, please.

On Sat, 1 Oct 2016, 22:29 Satrajit Ghosh, notifications@github.com wrote:

@alexsavio https://github.com/alexsavio - could we merge with current
master and make sure the tests pass for this.

—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
#1608 (comment), or mute
the thread
https://github.com/notifications/unsubscribe-auth/ACW4jJkt_COzBpchb0-hyyRyf5QW6M3dks5qvsKlgaJpZM4J4PUb
.

Sent from my phone, sorry for brevity or typos.

oesteban · 2016-10-04T16:49:17Z

This is a great addition!.

Please, merge #1627 first... :)

satra · 2016-10-06T10:59:14Z

@alexsavio - could you please merge with current master and resolve the conflicts.

alexsavio · 2016-10-06T11:34:17Z

Ok, sorry. I didn't get your previous message. Merged master.

oesteban · 2016-10-06T15:47:32Z

requirements.txt

@@ -8,6 +8,7 @@ nose>=1.2
 future==0.15.2
 simplejson>=3.8.0
 prov>=1.4.0
+click>=6.6.0


please, add 'click' in nipype/info.py, in the REQUIRES list.

oesteban · 2016-10-06T15:48:19Z

nipype/utils/nipype_cmd.py

-                        value = str2bool(value)
-                    except:
-                        pass
+    print("setting function inputs")


Can we use logging instead of prints?

I just copied this from the previous existing functions. Would you like me to use logging instead?

I think for the old nipype_cmd it makes sense. Probably it doesn't for nipype_display_crash and others.

Since this is previous code, I think I can do this in another PR.

oesteban · 2016-10-06T15:49:44Z

nipype/scripts/cli.py

+                                                ignore_template_numbers)
+
+    # Writes JSON string to file
+    with open(output, 'w') as f:


please add from io import open at the top (py2/3 compatibility)

* upstream/master: undo matplotlib change. move badge to codecov disable matplotlib extension to test doc building. merge upstream master fix: retroicor - cleaned up interface, vim presets

Nipypeclick

alexsavio · 2016-10-07T10:53:07Z

Thanks @satra

fix interface doc builder skip patterns

doc+fix: do not generate api from scripts directory

doc: update cli related docs

wip: move first nipype commands to a group command

4237c65

alexsavio added 2 commits September 9, 2016 17:10

wip: add "nipype run" command

578970e

remove unused function in scripts/instance

b2c9867

wip: add to_boutiques command

8207421

add convert subgroup for boutiques

64317f4

remove unused function

a40ee7e

satra mentioned this pull request Sep 12, 2016

ENH nipype_display_crash defaults to last created pkl file #1619

Closed

satra mentioned this pull request Sep 13, 2016

[ENH] Abandon distutils #1627

Merged

alexsavio added 2 commits September 16, 2016 10:46

fix CLI for print_usage commands fail print_help

029ddda

remove bin/ folder, replaced by click

d1cb857

remove unused import

44d78af

Merge remote-tracking branch 'nipype/master'

430761c

oesteban reviewed Oct 6, 2016

View reviewed changes

alexsavio and others added 6 commits October 6, 2016 17:52

add click to info.py REQUIRES

f17d8e2

fix missing import in cli.py

3d0c790

fix docstring in scripts.instance

a6697a8

Merge remote-tracking branch 'upstream/master' into nipypeclick

6689c1d

* upstream/master: undo matplotlib change. move badge to codecov disable matplotlib extension to test doc building. merge upstream master fix: retroicor - cleaned up interface, vim presets

add script installer

8f4e335

Merge pull request #11 from satra/nipypeclick

47e0291

Nipypeclick

satra and others added 2 commits October 7, 2016 11:04

fix interface doc builder skip patterns

d022077

Merge pull request #12 from satra/nipypeclick

e82fc40

fix interface doc builder skip patterns

alexsavio mentioned this pull request Oct 7, 2016

Add logging for nipype_cmd, replace print #1677

Open

satra and others added 4 commits October 7, 2016 15:49

doc+fix: do not generate api from scripts directory

2483b7a

Merge pull request #13 from satra/nipypeclick

1c1ef24

doc+fix: do not generate api from scripts directory

doc: update cli related docs

083e31b

Merge pull request #14 from satra/nipypeclick

3e650d4

doc: update cli related docs

satra approved these changes Oct 8, 2016

View reviewed changes

satra merged commit 865ad49 into nipy:master Oct 9, 2016

WIP: Move nipype commands to a group command #1608

WIP: Move nipype commands to a group command #1608

Uh oh!

Conversation

alexsavio commented Sep 8, 2016

Uh oh!

coveralls commented Sep 8, 2016

Uh oh!

coveralls commented Sep 8, 2016

Uh oh!

coveralls commented Sep 8, 2016

Uh oh!

coveralls commented Sep 8, 2016

Uh oh!

coveralls commented Sep 8, 2016

Uh oh!

coveralls commented Sep 8, 2016

Uh oh!

coveralls commented Sep 8, 2016 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

coveralls commented Sep 9, 2016

Uh oh!

coveralls commented Sep 9, 2016

Uh oh!

coveralls commented Sep 9, 2016

Uh oh!

coveralls commented Sep 9, 2016 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

coveralls commented Sep 9, 2016

Uh oh!

coveralls commented Sep 9, 2016

Uh oh!

coveralls commented Sep 9, 2016

Uh oh!

coveralls commented Sep 9, 2016

Uh oh!

coveralls commented Sep 9, 2016 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

alexsavio commented Sep 9, 2016

Uh oh!

coveralls commented Sep 9, 2016 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

coveralls commented Sep 9, 2016 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

alexsavio commented Sep 12, 2016 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

satra commented Sep 12, 2016

Uh oh!

alexsavio commented Sep 12, 2016

Uh oh!

codecov-io commented Sep 12, 2016 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Current coverage is 70.82% (diff: 29.91%)

Uh oh!

alexsavio commented Sep 19, 2016

Uh oh!

satra commented Oct 1, 2016

Uh oh!

alexsavio commented Oct 4, 2016

Uh oh!

oesteban commented Oct 4, 2016

Uh oh!

satra commented Oct 6, 2016

Uh oh!

alexsavio commented Oct 6, 2016

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

coveralls commented Sep 8, 2016 •

edited

Loading

coveralls commented Sep 9, 2016 •

edited

Loading

coveralls commented Sep 9, 2016 •

edited

Loading

coveralls commented Sep 9, 2016 •

edited

Loading

coveralls commented Sep 9, 2016 •

edited

Loading

alexsavio commented Sep 12, 2016 •

edited

Loading

codecov-io commented Sep 12, 2016 •

edited

Loading