Skip to content

Conversation

@maropu
Copy link
Member

@maropu maropu commented Jul 6, 2018

What changes were proposed in this pull request?

This pr supported column arguments in timezone of from_utc_timestamp/to_utc_timestamp (follow-up of #21693).

How was this patch tested?

Added tests.

@maropu
Copy link
Member Author

maropu commented Jul 6, 2018

@HyukjinKwon @gatorsmile

Copy link
Member

@HyukjinKwon HyukjinKwon left a comment

Choose a reason for hiding this comment

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

Seems from_utc_timestamp is missed.

return Column(sc._jvm.functions.to_utc_timestamp(_to_java_column(timestamp), tz))

if isinstance(tz, Column):
timezone = _to_java_column(tz)
Copy link
Member

Choose a reason for hiding this comment

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

@maropu, not a big deal at all but mind tz = _to_java_column(tz) to reduce the diff?

Copy link
Member Author

Choose a reason for hiding this comment

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

ok

"""
Given a timestamp like '2017-07-14 02:40:00.0', interprets it as a time in the given time
zone, and renders that time as a timestamp in UTC. For example, 'GMT+1' would yield
'2017-07-14 01:40:00.0'.
Copy link
Member

Choose a reason for hiding this comment

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

I have thought about this for long time and let's start to describe what we change within Python side:

        :param timestamp: the column that contains timestamps
        :param tz: a string that has the ID of timezone, e.g. "GMT", "America/Los_Angeles", etc.

        .. versionchanged:: 2.4
           `tz` can take a :class:`Column` containing timezone ID strings.

Copy link
Member Author

Choose a reason for hiding this comment

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

ok

@SparkQA
Copy link

SparkQA commented Jul 6, 2018

Test build #92667 has finished for PR 21723 at commit dfb5312.

  • This patch passes all tests.
  • This patch merges cleanly.
  • This patch adds no public classes.

@maropu
Copy link
Member Author

maropu commented Jul 6, 2018

oh, sorry, I'll update soon.

@SparkQA
Copy link

SparkQA commented Jul 6, 2018

Test build #92677 has finished for PR 21723 at commit 09911d5.

  • This patch passes all tests.
  • This patch merges cleanly.
  • This patch adds no public classes.

Copy link
Member

@HyukjinKwon HyukjinKwon left a comment

Choose a reason for hiding this comment

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

LGTM

@HyukjinKwon
Copy link
Member

@maropu, are you online? mind if I ask to fix PR title and description before I merge this one?

@maropu
Copy link
Member Author

maropu commented Jul 6, 2018

ah, ok. I’ll update soon

@maropu maropu changed the title [SPARK-24673][SQL][PYTHON][FOLLOWUP] Support column types in timezone of to_utc_timestamp [SPARK-24673][SQL][PYTHON][FOLLOWUP] Support Column arguments in timezone of from_utc_timestamp/to_utc_timestamp Jul 6, 2018
@maropu
Copy link
Member Author

maropu commented Jul 6, 2018

@HyukjinKwon ok, fixed!

@HyukjinKwon
Copy link
Member

Merged to master.

@HyukjinKwon
Copy link
Member

Thanks, @maropu.

@asfgit asfgit closed this in a381bce Jul 6, 2018
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.

3 participants