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.

Sign up for GitHub

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

Jump to bottom

forecaster.preprocess() may introduce unwanted behavior for NaN history values in 1.1.5 #2518

Closed
imad24 opened this issue Oct 25, 2023 · 2 comments · Fixed by #2530
Closed

forecaster.preprocess() may introduce unwanted behavior for NaN history values in 1.1.5 #2518

imad24 opened this issue Oct 25, 2023 · 2 comments · Fixed by #2530
Labels
bug py

Comments

@imad24
Copy link
Contributor

imad24 commented Oct 25, 2023 •
edited

I have noticed that the changes made to this line may introduce unwanted behavior in the future dataframe:

prophet/python/prophet/forecaster.py

Line 1133 in c00f6a2

self.history_dates = pd.to_datetime(pd.Series(history['ds'].unique(), name='ds')).sort_values()

This will make make_future_dataframe() ignore the dates with NaN values in the history, considering

history = df[df['y'].notnull()].copy()

in the versions prior to 1.1.5 (for reference):

self.history_dates = pd.to_datetime(pd.Series(df['ds'].unique(), name='ds')).sort_values()

Notice that the dataframe df['ds'].unique()is used instead of the not null history['ds'].unique()
@tcuongd
Copy link
Collaborator

tcuongd commented Oct 26, 2023

Thanks for noticing this! It makes sense that we'd want predictions for missing y dates in the history. More than happy to review a PR if you have time to raise a bug fix, otherwise I'll try get to it in the next couple of weeks.

@imad24
Copy link
Contributor Author

imad24 commented Nov 20, 2023

Hey @tcuongd,
I've created the PR to fix the bug and added a test for make_future_dataset(,,include_history=True)

@imad24 imad24 closed this as completed Nov 20, 2023
@imad24 imad24 reopened this Nov 20, 2023
@imad24 imad24 mentioned this issue Nov 20, 2023
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
bug py
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

Bug Fix: Include predictions for missing y (NaN) dates in the history #2518 imad24/prophet
2 participants
@imad24 @tcuongd