-
Notifications
You must be signed in to change notification settings - Fork 63
docs: Add time series analysis notebook #2328
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
Changes from 7 commits
2794d38
dc32fee
9c8c844
3f789f9
3bec4b3
bb691b0
51b336b
2528b86
85b99b3
b1215ca
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -17,6 +17,7 @@ | |
| from __future__ import annotations | ||
|
|
||
| from typing import List, Optional | ||
| import warnings | ||
|
|
||
| from google.cloud import bigquery | ||
|
|
||
|
|
@@ -230,6 +231,18 @@ def _fit( | |
| """ | ||
| X, y = utils.batch_convert_to_dataframe(X, y) | ||
|
|
||
| # Auto-convert Date to datetime for hourly/per_minute frequency | ||
| if self.data_frequency in ["hourly", "per_minute"]: | ||
| timestamp_col = X.columns[0] | ||
| if "date" in X[timestamp_col].dtype.name: | ||
| warnings.warn( | ||
| f"Converting Date column '{timestamp_col}' to datetime for " | ||
|
||
| f"{self.data_frequency} frequency. This is required because " | ||
| f"BigQuery ML doesn't support Date type with hourly frequency." | ||
| ) | ||
| X = X.copy() | ||
| X[timestamp_col] = bpd.to_datetime(X[timestamp_col]) | ||
|
|
||
| if X.columns.size != 1: | ||
| raise ValueError("Time series timestamp input X contain at least 1 column.") | ||
| if y.columns.size != 1: | ||
|
|
||
Large diffs are not rendered by default.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Non-rhetorical question: What if the the column has dtype "datetime"? Is it necessary that we still cast that column as datetime, and warn that the column has "date" type?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The error would not exist if column has a dtype "datetime". In this case, no cast/warning will exist.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hmmm, in that case the expression
would still evaluate to
True, right? Is that something we want?Uh oh!
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for suggestion. I believe Garrett has a fair point. Thus I revert the code change. Now this PR only include the notebook changes.