-
Notifications
You must be signed in to change notification settings - Fork 244
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
Update tests to not have hard coded fallback when not needed #10042
Conversation
Signed-off-by: Robert (Bobby) Evans <[email protected]>
build |
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.
I would just do the rename so this is a little more clear whether the time zone is supported, but otherwise it looks good.
from data_gen import * | ||
from datetime import date, datetime, timezone | ||
from marks import ignore_order, incompat, allow_non_gpu | ||
from pyspark.sql.types import * | ||
from spark_session import with_cpu_session, is_before_spark_330, is_before_spark_350 | ||
import pyspark.sql.functions as f | ||
|
||
# Some operations only work in UTC specifically | ||
non_utc_ts_allow = ['ProjectExec'] if not is_utc() else [] |
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_utc_ts_allow = ['ProjectExec'] if not is_utc() else [] | |
non_utc_tz_allow = ['ProjectExec'] if not is_utc() else [] |
# Some operations only work in UTC specifically | ||
non_utc_ts_allow = ['ProjectExec'] if not is_utc() else [] | ||
# Others work in all supported time zones | ||
non_supported_ts_allow = ['ProjectExec'] if not is_supported_time_zone() else [] |
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_supported_ts_allow = ['ProjectExec'] if not is_supported_time_zone() else [] | |
non_supported_tz_allow = ['ProjectExec'] if not is_supported_time_zone() else [] |
build |
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.
LGTM
This fixes the comments I made in #9816 (review) after the merge happened. Feedback is appreciated. I ran it manually with a number of time zones to verify that it works as expected.