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.

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

Merged
merged 2 commits into from
Dec 13, 2023

Conversation

revans2
Copy link
Collaborator

@revans2 revans2 commented Dec 13, 2023

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.

Signed-off-by: Robert (Bobby) Evans <bobby@apache.org>
@revans2
Copy link
Collaborator Author

revans2 commented Dec 13, 2023

build

Copy link
Collaborator

@NVnavkumar NVnavkumar left a 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 []
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
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 []
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
non_supported_ts_allow = ['ProjectExec'] if not is_supported_time_zone() else []
non_supported_tz_allow = ['ProjectExec'] if not is_supported_time_zone() else []

@revans2
Copy link
Collaborator Author

revans2 commented Dec 13, 2023

build

Copy link
Collaborator

@NVnavkumar NVnavkumar left a comment

Choose a reason for hiding this comment

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

LGTM

@revans2 revans2 merged commit 6145941 into NVIDIA:branch-24.02 Dec 13, 2023
37 of 38 checks passed
@revans2 revans2 deleted the fix_test_to_unix_ts branch December 13, 2023 21:08
@sameerz sameerz added the test Only impacts tests label Dec 16, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
test Only impacts tests
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants