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

MRG: Extend testing, fix / simplify interpolation #481

Merged
merged 1 commit into from Mar 10, 2021

Conversation

matthew-brett
Copy link
Member

Extend testing of prefiltered interpolation.

This revealed various bugs:

  • Prefiltering=False of linear interpolation was failing, because the
    prefilter=False option didn't make sense - because we were (correctly)
    not prefiltering.
  • Use more standard TemporaryFile for backing store to avoid errors from
    too many open files.
  • Avoid padding before prefiltering for Scipy < 1.6, because this gives
    different results from non-prefiltered case.

Also see #478

@coveralls
Copy link

coveralls commented Mar 2, 2021

Coverage Status

Coverage increased (+0.02%) to 86.372% when pulling dfbac87 on matthew-brett:testing-interpolate into 3c06229 on nipy:master.

@codecov
Copy link

codecov bot commented Mar 2, 2021

Codecov Report

Merging #481 (dfbac87) into master (3c06229) will increase coverage by 0.02%.
The diff coverage is 94.87%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #481      +/-   ##
==========================================
+ Coverage   83.00%   83.02%   +0.02%     
==========================================
  Files         306      306              
  Lines       28441    28448       +7     
  Branches     3303     3304       +1     
==========================================
+ Hits        23608    23620      +12     
+ Misses       3825     3821       -4     
+ Partials     1008     1007       -1     
Impacted Files Coverage Δ
nipy/algorithms/interpolation.py 94.00% <83.33%> (+5.94%) ⬆️
nipy/algorithms/tests/test_interpolator.py 100.00% <100.00%> (ø)

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 3c06229...dfbac87. Read the comment docs.

Extend testing of prefiltered interpolation.

This revealed various bugs:

* Prefiltering=False of linear interpolation was failing, because the
  prefilter=False option didn't make sense - because we were (correctly)
  not prefiltering.
* Use more standard TemporaryFile for backing store to avoid errors from
  too many open files.
* Avoid padding before prefiltering for Scipy < 1.6, because this gives
  different results from non-prefiltered case.
@matthew-brett
Copy link
Member Author

I think this is the right fix, with extra tests, for the issue in #478 - as well as a couple of extra issues I found during testing.

@matthew-brett
Copy link
Member Author

Last call for a review here?

@matthew-brett
Copy link
Member Author

Thanks Chris.

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.

None yet

3 participants