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

New module: dipy.core.interpolation #1896

Merged
merged 14 commits into from Jul 24, 2019

Conversation

@skoudoro
Copy link
Member

commented Jul 11, 2019

The goal of this PR is to bring all our interpolation functions in one place. By doing that, It seems that some functions are doing exactly the same thing. This part needs a clean up on a new PR.

This PR should fix #728.

Let me know if there is an interpolation function that I forgot to move in this module

@arokem

This comment has been minimized.

Copy link
Member

commented Jul 11, 2019

Do you want to include this one?

def interp_rbf(data, sphere_origin, sphere_target,

@skoudoro

This comment has been minimized.

Copy link
Member Author

commented Jul 11, 2019

good catch! let's add this function too!

@skoudoro skoudoro added this to the 1.0 milestone Jul 11, 2019

@arokem

This comment has been minimized.

Copy link
Member

commented Jul 15, 2019

This looks overall good to me.

@skoudoro : could you please rebase and resolve conflicts?

@arokem

This comment has been minimized.

Copy link
Member

commented Jul 15, 2019

Once you rebase, I think that the Appveyor failer should go away, as #1899 is now merged.

@skoudoro skoudoro force-pushed the skoudoro:interpolation-module branch from a57130b to aee04cc Jul 16, 2019

@codecov-io

This comment has been minimized.

Copy link

commented Jul 16, 2019

Codecov Report

❗️ No coverage uploaded for pull request base (master@ff5aa84). Click here to learn what that means.
The diff coverage is 100%.

Impacted file tree graph

@@            Coverage Diff            @@
##             master    #1896   +/-   ##
=========================================
  Coverage          ?   85.41%           
=========================================
  Files             ?      118           
  Lines             ?    14251           
  Branches          ?     2238           
=========================================
  Hits              ?    12173           
  Misses            ?     1573           
  Partials          ?      505
Impacted Files Coverage Δ
dipy/core/sphere.py 97.43% <ø> (ø)
dipy/tracking/streamline.py 92.95% <100%> (ø)
dipy/align/imaffine.py 92.07% <100%> (ø)
@skoudoro

This comment has been minimized.

Copy link
Member Author

commented Jul 16, 2019

rebased and no Appveyor error anymore. this PR is ready.

Comments are welcomed!

@skoudoro skoudoro force-pushed the skoudoro:interpolation-module branch from aee04cc to 74a0f73 Jul 18, 2019

@Garyfallidis

This comment has been minimized.

Copy link
Member

commented Jul 24, 2019

Great, finally we have one place for all interpolation functions. In the future we may want to separate interpolation.pyx to interpolation.py and interpolation_speed.pyx but no need to this now.

@Garyfallidis Garyfallidis merged commit f059728 into nipy:master Jul 24, 2019

0 of 2 checks passed

continuous-integration/appveyor/pr Waiting for AppVeyor build to complete
Details
continuous-integration/travis-ci/pr The Travis CI build is in progress
Details
@Garyfallidis

This comment has been minimized.

Copy link
Member

commented Jul 24, 2019

Thank you @skoudoro. Please everyone rebase!

@skoudoro skoudoro deleted the skoudoro:interpolation-module branch Jul 24, 2019

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
4 participants
You can’t perform that action at this time.