Skip to content

RF: Redirect nipype.workflows to niflow.nipype1.workflows #3067

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
Oct 8, 2019

Conversation

effigies
Copy link
Member

@effigies effigies commented Oct 7, 2019

Summary

Imports from niflow-nipype1-workflows, adding the available modules to sys.modules under their old names, so that code that currently imports via import nipype.workflows.X or from nipype.workflows.Y import Z should continue to work.

Closes #2991.

Acknowledgment

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

@codecov
Copy link

codecov bot commented Oct 7, 2019

Codecov Report

Merging #3067 into master will increase coverage by 0.48%.
The diff coverage is 54.28%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #3067      +/-   ##
==========================================
+ Coverage    67.7%   68.19%   +0.48%     
==========================================
  Files         344      297      -47     
  Lines       44112    39775    -4337     
  Branches     5563     5211     -352     
==========================================
- Hits        29866    27124    -2742     
+ Misses      13481    11941    -1540     
+ Partials      765      710      -55
Flag Coverage Δ
#smoketests 51.78% <48.57%> (+1.48%) ⬆️
#unittests 65.44% <54.28%> (+0.32%) ⬆️
Impacted Files Coverage Δ
nipype/interfaces/fsl/epi.py 63.22% <ø> (ø) ⬆️
nipype/interfaces/fsl/dti.py 65.79% <ø> (ø) ⬆️
nipype/workflows/__init__.py 100% <100%> (ø)
nipype/interfaces/mrtrix/convert.py 17.85% <11.11%> (-1.14%) ⬇️
nipype/pipeline/engine/workflows.py 76.93% <0%> (-2.03%) ⬇️
nipype/interfaces/freesurfer/base.py 78.81% <0%> (-1.7%) ⬇️
nipype/interfaces/fsl/base.py 86.66% <0%> (-1.12%) ⬇️
nipype/interfaces/fsl/preprocess.py 81.76% <0%> (-0.29%) ⬇️
nipype/interfaces/dipy/tracks.py 32.22% <0%> (+0.55%) ⬆️
... and 1 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 1ccd70b...5c9e571. Read the comment docs.

@effigies effigies requested review from mgxd and satra October 7, 2019 21:45
@effigies
Copy link
Member Author

effigies commented Oct 7, 2019

I feel like this would be a good one to get in for RC1, as it has the chance to break things for people, so finding those before the final release would be good.

@satra
Copy link
Member

satra commented Oct 7, 2019

i like it and a question.

how are we dealing with circularity? we are installing nipype and these niflows require nipype.

@effigies
Copy link
Member Author

effigies commented Oct 7, 2019

Niflows depend on nipype, nipype will detect them at runtime, so there isn't an installation circularity.

For the purpose of testing on docker, it installs the latest release and then installs the current source on top of that. We could just as easily post-install the niflows.

Copy link
Member

@mgxd mgxd left a comment

Choose a reason for hiding this comment

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

LGTM, would be nice to include an extra_requires that automatically installs niflow.nipype1.workflows but this might be circular

@effigies
Copy link
Member Author

effigies commented Oct 8, 2019

Yeah, that would be circular, though it's possible pip is more relaxed for extras. I also don't want to encourage people to continue using this API, so making it a little difficult is fine with me.

@effigies effigies mentioned this pull request Oct 8, 2019
13 tasks
@effigies effigies added this to the 1.3.0 milestone Oct 8, 2019
@effigies effigies merged commit 0e11cea into nipy:master Oct 8, 2019
@effigies effigies deleted the mnt/workflows_to_niflows branch October 8, 2019 03:00
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.

Replace nipype.workflows with a shim that imports niflow-nipype1-workflows
3 participants