Skip to content

ENH: ewm() default to using DatetimeIndex as times when halflife is a timedelta #54181

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

Closed
wants to merge 2 commits into from

Conversation

gsiano
Copy link
Contributor

@gsiano gsiano commented Jul 18, 2023

Allow ewm() to use a DatetimeIndex as the times parameter if it isn't provided and halflife is a timedelta convertible.

Before:

>>> now = pd.Timestamp.now()
>>> idx = [now + pd.Timedelta(seconds=i) for i in range(1, 4)]
>>> df = pd.DataFrame({'a': [1,2,3]}, index=idx)
>>> df.ewm(halflife=pd.Timedelta(seconds=1)).mean()
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
  File "/home/gsiano/.local/lib/python3.10/site-packages/pandas/core/generic.py", line 11743, in ewm
    return ExponentialMovingWindow(
  File "/home/gsiano/.local/lib/python3.10/site-packages/pandas/core/window/ewm.py", line 386, in __init__
    raise ValueError(
ValueError: halflife can only be a timedelta convertible argument if times is not None.
>>> df.ewm(halflife=pd.Timedelta(seconds=1), times=df.index).mean()
                                   a
2023-07-18 10:45:18.007264  1.000000
2023-07-18 10:45:19.007264  1.666667
2023-07-18 10:45:20.007264  2.428571
>>> 

After:

>>> now = pd.Timestamp.now()
>>> idx = [now + pd.Timedelta(seconds=i) for i in range(1, 4)]
>>> df = pd.DataFrame({'a': [1,2,3]}, index=idx)
>>> df.ewm(halflife=pd.Timedelta(seconds=1)).mean()
                                   a
2023-07-18 10:46:46.589891  1.000000
2023-07-18 10:46:47.589891  1.666667
2023-07-18 10:46:48.589891  2.428571
  • closes #xxxx (Replace xxxx with the GitHub issue number)
  • Tests added and passed if fixing a bug or adding a new feature
  • All code checks passed.
  • Added type annotations to new arguments/methods/functions.
  • Added an entry in the latest doc/source/whatsnew/vX.X.X.rst file if fixing a bug or adding a new feature.

Copy link
Member

@mroeschke mroeschke left a comment

Choose a reason for hiding this comment

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

This would be an API behavior change so this isn't possible as is. Was this discussed in an issue?

@gsiano
Copy link
Contributor Author

gsiano commented Jul 18, 2023

This would be an API behavior change so this isn't possible as is. Was this discussed in an issue?

No, it wasn't. Should I create an issue? Or is something like this not worth an API change?

@mroeschke
Copy link
Member

You could create an issue, but IMO this behavior is less explicit so IMO I am not sure this is a good change

@mroeschke
Copy link
Member

Thanks for opening up #54187, going to close this PR for now until there has been more opt-in from other core developers in the issue

@mroeschke mroeschke closed this Jul 19, 2023
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.

2 participants