Skip to content
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

FIX: Restore AFNICommand._get_fname, required by some interfaces #3071

Merged
merged 1 commit into from
Oct 8, 2019

Conversation

oesteban
Copy link
Contributor

@oesteban oesteban commented Oct 7, 2019

In #2964 I broke some interfaces (@tsalo identified Allineate as one
of those) by removing the _get_fname function from the base
AFNICommand.

The intent was to migrate to a name_source-based management of
automatically generated names, but in the case of Allineate, the use
of _get_fname was a bit different (modifying the user-provide input
value under certain conditions to follow ANFI's 3dAllineate
behavior).

This PR restores the missing function. Closes #3070.

Summary

Fixes # .

List of changes proposed in this PR (pull-request)

Acknowledgment

  • (Mandatory) I acknowledge that this contribution will be available under the Apache 2 license.

@effigies
Copy link
Member

effigies commented Oct 7, 2019

Should we re-add to just Allineate? I suggest this from my phone, without looking closely at it.

In nipy#2964 I broke some interfaces (@tsalo identified `Allineate` as one
of those) by removing the ``_get_fname`` function from the base
``AFNICommand``.

The intent was to migrate to a ``name_source``-based management of
automatically generated names, but in the case of ``Allineate``, the use
of `_get_fname` was a bit different (modifying the user-provide input
value under certain conditions to follow ANFI's ``3dAllineate``
behavior).

Also remove a definition of the same function for ``Deconvolve``.

This PR restores the missing function. Closes nipy#3070.
@codecov
Copy link

codecov bot commented Oct 7, 2019

Codecov Report

Merging #3071 into master will decrease coverage by 3.52%.
The diff coverage is 37.5%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #3071      +/-   ##
==========================================
- Coverage    67.7%   64.17%   -3.53%     
==========================================
  Files         344      342       -2     
  Lines       44111    44027      -84     
  Branches     5563     5552      -11     
==========================================
- Hits        29866    28255    -1611     
- Misses      13480    14653    +1173     
- Partials      765     1119     +354
Flag Coverage Δ
#smoketests ?
#unittests 64.17% <37.5%> (-0.96%) ⬇️
Impacted Files Coverage Δ
nipype/interfaces/afni/model.py 87% <ø> (+4.95%) ⬆️
nipype/interfaces/afni/base.py 67.76% <37.5%> (-4.88%) ⬇️
nipype/interfaces/nilearn.py 40% <0%> (-56.67%) ⬇️
nipype/utils/spm_docs.py 25.92% <0%> (-44.45%) ⬇️
nipype/interfaces/freesurfer/base.py 50% <0%> (-30.51%) ⬇️
nipype/utils/logger.py 59.7% <0%> (-29.86%) ⬇️
nipype/algorithms/rapidart.py 35% <0%> (-29.42%) ⬇️
nipype/interfaces/spm/base.py 57.94% <0%> (-29.14%) ⬇️
nipype/utils/provenance.py 55.73% <0%> (-28.35%) ⬇️
nipype/interfaces/fsl/model.py 55.26% <0%> (-25.35%) ⬇️
... and 43 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 cffb1f2...e6a1cc7. Read the comment docs.

@oesteban
Copy link
Contributor Author

oesteban commented Oct 7, 2019

Unfortunately, a few more interfaces also got orphaned. I don't think it would be hard to migrate them to name_sources (esp. those under afni.utils), but I guess that's material for a different PR.

I also found that Deconvolve was defining the function, so removed it.

@oesteban oesteban added this to the 1.3.0 milestone Oct 8, 2019
@effigies
Copy link
Member

effigies commented Oct 8, 2019

Sounds good.

@effigies effigies mentioned this pull request Oct 8, 2019
13 tasks
@effigies effigies merged commit 5976769 into nipy:master Oct 8, 2019
@oesteban oesteban deleted the fix/3070 branch October 8, 2019 05:51
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.

Removal of AFNICommand._gen_fname breaks Allineate
2 participants