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

BUG: core.missing._akima_interpolate will raise AttributeError #33426

Closed
jbrockmendel opened this issue Apr 9, 2020 · 8 comments · Fixed by #33784
Closed

BUG: core.missing._akima_interpolate will raise AttributeError #33426

jbrockmendel opened this issue Apr 9, 2020 · 8 comments · Fixed by #33784
Assignees
Labels
Bug good first issue Needs Tests Unit test(s) needed to prevent regressions
Milestone

Comments

@jbrockmendel
Copy link
Member

    from scipy import interpolate

    P = interpolate.Akima1DInterpolator(xi, yi, axis=axis)

    if der == 0:
        return P(x)
    elif interpolate._isscalar(der):
        return P(x, der=der)
    else:
        return [P(x, nu) for nu in der]

There is no interpolate._isscalar so this would raise if it were ever reached. AFAICT we have only one test that gets to this function, must always have der == 0

@jbrockmendel jbrockmendel added Bug Needs Triage Issue that has not been reviewed by a pandas team member labels Apr 9, 2020
@jbrockmendel
Copy link
Member Author

I'm guessing this is the function that we're trying to get at:

def _isscalar(x):
    """Check whether x is if a scalar type, or 0-dim"""
    return np.isscalar(x) or hasattr(x, 'shape') and x.shape == ()

@jbrockmendel jbrockmendel added good first issue Needs Tests Unit test(s) needed to prevent regressions and removed Needs Triage Issue that has not been reviewed by a pandas team member labels Apr 14, 2020
@fifthist
Copy link

fifthist commented Apr 15, 2020

We could

  1. Import _isscalar directly inside the definition of _akima_interpolate.
from scipy.interpolate.polyint import _isscalar
  1. Use is_scalar, which is already imported in the module. Looking at it's definition, it seems to check both cases that _isscalar checks. However, this considers datetime objects and None to be scalars, so it may be too nonrestrictive.

@jbrockmendel
Copy link
Member Author

we'll want to use is_scalar, will probably also need a lib.item_from_zerodim.

@hkennyv
Copy link
Contributor

hkennyv commented Apr 23, 2020

take

hkennyv added a commit to hkennyv/pandas that referenced this issue Apr 23, 2020
The TypeError was in __call__ due to using wrong kwarg `der` instead of `nu`.
@hkennyv
Copy link
Contributor

hkennyv commented Apr 23, 2020

Hey all, I'm trying to add to the test_interpolate_akima test so that it covers cases where der is a list but I'm having trouble debugging this error:

Exception has occurred: TypeError
NumPy boolean array indexing assignment requires a 0 or 1-dimensional input, input has 2 dimensions
  File "/Users/khuynh/github/pandas/pandas/core/missing.py", line 303, in interpolate_1d
    result[invalid] = _interpolate_scipy_wrapper(
  File "/Users/khuynh/github/pandas/pandas/core/internals/blocks.py", line 1203, in func
    return missing.interpolate_1d(
  File "<__array_function__ internals>", line 5, in apply_along_axis
  File "/Users/khuynh/github/pandas/pandas/core/internals/blocks.py", line 1216, in _interpolate
    interp_values = np.apply_along_axis(func, axis, data)
  File "/Users/khuynh/github/pandas/pandas/core/internals/blocks.py", line 1113, in interpolate
    return self._interpolate(
  File "/Users/khuynh/github/pandas/pandas/core/internals/managers.py", line 397, in apply
    applied = getattr(b, f)(**kwargs)
  File "/Users/khuynh/github/pandas/pandas/core/internals/managers.py", line 558, in interpolate
    return self.apply("interpolate", **kwargs)
  File "/Users/khuynh/github/pandas/pandas/core/generic.py", line 6897, in interpolate
    new_data = data.interpolate(
  File "/Users/khuynh/github/pandas/test.py", line 33, in <module>
    interp_s = ser.reindex(new_index).interpolate(method="akima", der=[1, 2])

I'm getting this error when running this test file I made so I could step through with my debugger (using the editable install btw, you'll run into other errors if you run it with the current master. see #33426 6c42496 and #33426 a147bb0):

import numpy as np
import pytest

import pandas.util._test_decorators as td

import pandas as pd
from pandas import Index, MultiIndex, Series, date_range, isna
import pandas._testing as tm

print("hello world!")

ser = Series([10, 11, 12, 13])

expected = Series(
    [11.00, 11.25, 11.50, 11.75, 12.00, 12.25, 12.50, 12.75, 13.00],
    index=Index([1.0, 1.25, 1.5, 1.75, 2.0, 2.25, 2.5, 2.75, 3.0]),
)
# interpolate at new_index
new_index = ser.index.union(Index([1.25, 1.5, 1.75, 2.25, 2.5, 2.75])).astype(
    float
)
interp_s = ser.reindex(new_index).interpolate(method="akima")
# tm.assert_series_equal(interp_s[1:3], expected)
print(interp_s)

# test interpolate when a non-zero int
interp_s = ser.reindex(new_index).interpolate(method="akima", der=1)
# tm.assert_series_equal(interp_s[1:3], expected)
print(interp_s)
print(interp_s[1:3])

# test interpolate when der is a list
interp_s = ser.reindex(new_index).interpolate(method="akima", der=[1, 2])
# tm.assert_series_equal(interp_s[1:3], expected)

print(interp_s)
print('success!')

Any pointers?

@jbrockmendel
Copy link
Member Author

test so that it covers cases where der is a list

why?

@hkennyv
Copy link
Contributor

hkennyv commented Apr 24, 2020

because the docstring for core.missing._akima_interpolate specififes der to be "int or list". But I see upon checking scipy.interpolate.PPoly.__call__ that the docstring is incorrect.

I can modify the docstring for _akima_interpolate and remove that else branch and submit a PR if everything else checks out?

@jbrockmendel
Copy link
Member Author

I can modify the docstring for _akima_interpolate and remove that else branch and submit a PR if everything else checks out?

sounds good

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Bug good first issue Needs Tests Unit test(s) needed to prevent regressions
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants