FIX: Build cmdline from working directory #2521
Conversation
effigies
force-pushed
the
fix/cmdline_cwd
branch
from
067dba9
to
5c10c92
Compare
Codecov Report
@@ Coverage Diff @@
## master #2521 +/- ##
==========================================
+ Coverage 66.84% 66.88% +0.04%
==========================================
Files 327 327
Lines 42479 42487 +8
Branches 5268 5268
==========================================
+ Hits 28395 28419 +24
+ Misses 13380 13366 -14
+ Partials 704 702 -2
Continue to review full report at Codecov.
|
| @@ -627,7 +627,8 @@ def _run_command(self, execute, copyfiles=True): | |||
| self._interface.__class__.__name__) | |||
| if issubclass(self._interface.__class__, CommandLine): | |||
| try: | |||
| cmd = self._interface.cmdline | |||
| with indirectory(outdir): | |||
There was a problem hiding this comment.
does outdir always exist at this point?
There was a problem hiding this comment.
We create the command.txt file in line 637, just below here, and copy files to working directory just above.
A quick scan didn't show me where the directory is created, but I'm reasonable confident it exists.
There was a problem hiding this comment.
The directory is created here, inside run():
Therefore, it is not granted that it will exist at this point. We could add a makedirs call to the context manager.
There was a problem hiding this comment.
run() then calls _run_interface(), which calls _run_command(), which contains this code.
There was a problem hiding this comment.
but you can always call cmdline manually before run.
inside a workflow this does not apply, but many people use the interfaces and call cmdline in their ipython interactive sessions.
|
This is for when a node is run, not an interface. Dropping files in the
local cwd is standard for interfaces.
…On Sat, Mar 31, 2018, 23:04 Oscar Esteban ***@***.***> wrote:
***@***.**** commented on this pull request.
------------------------------
In nipype/pipeline/engine/nodes.py
<#2521 (comment)>:
> @@ -627,7 +627,8 @@ def _run_command(self, execute, copyfiles=True):
self._interface.__class__.__name__)
if issubclass(self._interface.__class__, CommandLine):
try:
- cmd = self._interface.cmdline
+ with indirectory(outdir):
but you can always call cmdline manually before run.
inside a workflow this does not apply, but many people use the interfaces
and call cmdline in their ipython interactive sessions.
—
You are receiving this because you authored the thread.
Reply to this email directly, view it on GitHub
<#2521 (comment)>, or mute
the thread
<https://github.com/notifications/unsubscribe-auth/AAFF8qDan3y-y14qlf999Qzbpfv7EiYqks5tkEPRgaJpZM4TCAnK>
.
|
|
@effigies - that is indeed an important conceptual distinction. interfaces do whatever the underlying command would do without trying to change the behavior of the command. nodes have additional policies (fixed working directory), which is why interfaces have some additional metadata for nodes ( |
|
Not quite sure if we're all on the same page here. Just to clarify: It is my position that this doesn't need changing at this point. @satra @oesteban If you disagree or are expecting changes, please let me know. |
|
Gotcha, no disagreement here |
This fix supports an earlier ridiculous hack (#1857) that was broken by properly managing working directories (#2368).
Fixes #2518.
Changes proposed in this pull request
cmdlinewithin the working directory, for the probably one case in which thecmdlinedepends on the working directory.indirectorycontext manager, because I hate theos.getcwd(); os.chdir(); CONTENT; os.chdir()pattern.There may be a more elegant way to get input traits into a file, but it's not clear to me what that is at this point, so I went for the cheap solution.