Conversation

oesteban

Another PR removing stuff from the interface class definition. It also adds more precise exceptions.

Another PR removing stuff from the interface class definition.
@codecov-io

Codecov Report

Merging #2799 into master will decrease coverage by 3.34%.
The diff coverage is 95.93%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #2799      +/-   ##
==========================================
- Coverage   67.52%   64.18%   -3.35%     
==========================================
  Files         341      340       -1     
  Lines       43312    43324      +12     
  Branches     5371     5379       +8     
==========================================
- Hits        29248    27808    -1440     
- Misses      13363    14443    +1080     
- Partials      701     1073     +372
FlagCoverage Δ
#smoketests?
#unittests64.18% <95.93%> (-0.77%)⬇️
Impacted FilesCoverage Δ
nipype/interfaces/fsl/utils.py63.8% <ø> (-6.53%)⬇️
nipype/info.py88.23% <100%> (-5.89%)⬇️
nipype/utils/errors.py100% <100%> (ø)
nipype/interfaces/base/core.py84.65% <100%> (-2.44%)⬇️
nipype/interfaces/freesurfer/preprocess.py64.5% <33.33%> (-1.61%)⬇️
nipype/interfaces/base/specs.py89.01% <95.65%> (-4.07%)⬇️
nipype/interfaces/nilearn.py40% <0%> (-56.67%)⬇️
nipype/utils/spm_docs.py25.92% <0%> (-44.45%)⬇️
nipype/interfaces/freesurfer/base.py50% <0%> (-30.51%)⬇️
nipype/utils/logger.py59.7% <0%> (-29.86%)⬇️
... and 50 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 0fde422...2d5fd05. Read the comment docs.

@oesteban

There is one important change in this PR: the cmdline property of CommandLineInterface does not raise exceptions anymore, just warnings. When cmdline is not ready (e.g., because of missing mandatory inputs or conflicted xors) now it returns None and prints a warning.

The rationale is that, if the user wants to check the inputs, the appropriate way of doing it would be using the function nipype.interfaces.base.specs.check_mandatory_inputs(). After all, getting a None when calling cmdline is a strong signal of the problem, since there is no other use case where cmdline can be None. Hence it is not necessary to raise errors, which may be disruptive if used unnecessarily.

@effigieseffigies added this to the 1.1.7 milestone Nov 26, 2018
@oestebanoesteban modified the milestone: 1.1.7 Nov 26, 2018
@effigieseffigies modified the milestones: 1.1.7, 1.1.8 Dec 14, 2018
@effigies

Hi, just a reminder that the 1.1.8 release is targeted for January 28. Please let us know if you'd like to try to finish this up for that release.

@oesteban

This one would be good to have in 1.1.8. I'll try to resolve conflicts ASAP

@effigies

If you can wait on #2855, that should fix Travis.

@effigieseffigies mentioned this pull request Jan 25, 2019
16 tasks
@effigieseffigies modified the milestones: 1.1.8, future Jan 27, 2019
@effigies

This pull request is "orphaned," which means it has been deemed to be abandoned by its original author. Orphaned pull requests have not been rejected, and we hope that if a user sees one that will meet their needs with a little work, that they will fork it and open a new pull request (or, in the case of the original author, reopen the original PR).

We ask that all adopted PRs be updated to merge or rebase the current master. If you would like to adopt a PR and need help getting started, any of a number of contributors will be happy to help.

Sign up for free to join this conversation on . Already have an account? Sign in to comment
None yet

Successfully merging this pull request may close these issues.