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

ENH: interpolate: allow Akima extrapolation #20304

Merged
merged 6 commits into from May 8, 2024
Merged

Conversation

psmd-iberutaru
Copy link
Contributor

Reference issue

Minor change. Closes #20247

What does this implement/fix?

We add the extrapolation parameter to the construction; passing it to the parent classes. We keep False as the default as that is the behavior of the function before this change.

Additional information

@github-actions github-actions bot added scipy.interpolate enhancement A new feature or improvement labels Mar 21, 2024
Copy link
Member

@j-bowhay j-bowhay left a comment

Choose a reason for hiding this comment

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

Thanks @psmd-iberutaru can you add some test coverage for this new functionality

@psmd-iberutaru
Copy link
Contributor Author

Apologies @j-bowhay. Added the missing unit test(s). If there is a missing test, please let me know so I may add it.

@fancidev
Copy link
Contributor

I guess the doc of the argument “extrapolate” should read “If bool, …” instead of “If true, …” to be consistent with the doc of CubicHermiteSpline and PPoly. @psmd-iberutaru

@psmd-iberutaru
Copy link
Contributor Author

Just a follow up ping for review: @ev-br @j-bowhay

Copy link
Member

@ev-br ev-br left a comment

Choose a reason for hiding this comment

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

LGTM modulo a couple of tolerances in tests. Generally, it's better to explicitly set e.g. assert_allclose(x, y, atol=1e-15) to signal equality in the floating point.

scipy/interpolate/tests/test_interpolate.py Outdated Show resolved Hide resolved
ak_none = Akima1DInterpolator(x, y, extrapolate=None)
# None should default to False; extrapolated points are NaN.
assert_allclose(ak_false(x_ext), ak_none(x_ext), equal_nan=True)
assert_equal(ak_false(x_ext)[0:4], np.full(4, np.nan))
Copy link
Member

Choose a reason for hiding this comment

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

TIL that assert_equal(np.nan, np.nan) is True not False.

scipy/interpolate/tests/test_interpolate.py Outdated Show resolved Hide resolved
scipy/interpolate/tests/test_interpolate.py Outdated Show resolved Hide resolved
@ev-br ev-br added this to the 1.14.0 milestone May 7, 2024
@ev-br
Copy link
Member

ev-br commented May 7, 2024

Jake @j-bowhay , your review is red on GH, is there something you'd like to see in this PR?

@j-bowhay j-bowhay dismissed their stale review May 7, 2024 16:52

Past comment resolved, have not had time to review again

@lucascolley lucascolley changed the title ENH: Allow Akima extrapolation ENH: interpolate: allow Akima extrapolation May 7, 2024
@ev-br ev-br merged commit 0ca4d77 into scipy:main May 8, 2024
30 checks passed
@ev-br
Copy link
Member

ev-br commented May 8, 2024

CI is green, review comments are addressed, the patch LGTM and is merged. Thank you @psmd-iberutaru, all. And congrats with your first scipy contribution! Keep them coming :-).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement A new feature or improvement scipy.interpolate
Projects
None yet
Development

Successfully merging this pull request may close these issues.

ENH: Akima1DInterpolator Extrapolation
4 participants