bashtage / arch

ARCH models in Python
Other
1.32k stars 245 forks source link

Fixed deprecation warning #682

Closed jorenham closed 11 months ago

jorenham commented 11 months ago

This fixes several cases of:

FutureWarning: Series.__getitem__ treating keys as positions is deprecated. In a future version, integer keys will always be treated as labels (consistent with DataFrame behavior). To access a value by position, use `ser.iloc[pos]`
pep8speaks commented 11 months ago

Hello @jorenham! Thanks for updating this PR. We checked the lines you've touched for PEP 8 issues, and found:

There are currently no PEP 8 issues detected in this Pull Request. Cheers! :beers:

Comment last updated at 2023-09-27 16:15:58 UTC
codecov[bot] commented 11 months ago

Codecov Report

All modified lines are covered by tests :white_check_mark:

Files Coverage Δ
arch/tests/univariate/test_forecast.py 100.00% <100.00%> (ø)
arch/tests/univariate/test_mean.py 100.00% <100.00%> (ø)
arch/tests/univariate/test_rescale.py 100.00% <100.00%> (ø)
arch/unitroot/_engle_granger.py 100.00% <100.00%> (ø)
arch/unitroot/unitroot.py 99.86% <100.00%> (ø)
arch/univariate/base.py 98.93% <100.00%> (-0.02%) :arrow_down:

:loudspeaker: Thoughts on this report? Let us know!.

bashtage commented 11 months ago

I think it would be better to explicitly use .iloc rather can converting the entire array.

There are quite a few in the test log. Feel like tackling the others?

jorenham commented 11 months ago

@bashtage I wanted to to that before, but after looking at the statsmodels LikelihoodModelResults class, I saw that params, and therefore tvalues, could also be a numpy array. This way it'll work in both cases.

I'll look into the remaining warnings as well.

jorenham commented 11 months ago

@bashtage I'm not sure why this test is failing. https://dev.azure.com/kevinksheppard0207/kevinksheppard/_build/results?buildId=944&view=logs&j=7cb0d55f-58f1-52d4-9308-3ef157e47daa&t=56ed1dc5-eb59-5286-8983-2e32283fa027&l=93

jorenham commented 11 months ago

I think it would be better to explicitly use .iloc rather can converting the entire array.

There are quite a few in the test log. Feel like tackling the others?

For series of length 2, it appears that np.asarray(params)[0] is a bit faster than params.iloc[0] (1.4µs vs 1.6µs), using pandas 2.1 on python 3.11. But I found a better solution, param0, _* = params, which runs on average in 430 ns, which also works for pandas.Series and np.ndarray objects.

So I'll go ahead and update the changes to this unpacking method.

bashtage commented 11 months ago

Looks like 2 final one:

rch/tests/univariate/test_mean.py::test_param_cov
  /opt/hostedtoolcache/Python/3.11.5/x64/lib/python3.11/site-packages/statsmodels/tools/numdiff.py:150: FutureWarning: Series.__getitem__ treating keys as positions is deprecated. In a future version, integer keys will always be treated as labels (consistent with DataFrame behavior). To access a value by position, use `ser.iloc[pos]`
    ei[k] = epsilon[k]

arch/tests/univariate/test_mean.py::test_param_cov
  /opt/hostedtoolcache/Python/3.11.5/x64/lib/python3.11/site-packages/statsmodels/tools/numdiff.py:151: FutureWarning: Series.__getitem__ treating keys as positions is deprecated. In a future version, integer keys will always be treated as labels (consistent with DataFrame behavior). To access a value by position, use `ser.iloc[pos]`
    grad[k, :] = (f(*((x+ei,) + args), **kwargs) - f0)/epsilon[k]
bashtage commented 11 months ago

Thanks. Going ot merge this. I'll catch the other two.