Skip to content

Commit 863cdd4

Browse files
authored
GH-33143: [C++] Naming and doc/test changes for local_time compute kernel (#34263)
### Rationale for this change A better naming for local_time kernel was proposed in post merge review of #34208. ### What changes are included in this PR? Change `local_time` to `local_timestamp` and related docs/test changes. ### Are these changes tested? Yes. ### Are there any user-facing changes? Changing `local_time` to `local_timestamp` is a user facing change. But since it was not yet released we can probably treat it as non-breaking. * Closes: #33143 Authored-by: Rok Mihevc <rok@mihevc.org> Signed-off-by: Rok Mihevc <rok@mihevc.org>
1 parent e0e740b commit 863cdd4

6 files changed

Lines changed: 40 additions & 37 deletions

File tree

cpp/src/arrow/compute/api_scalar.cc

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -834,7 +834,7 @@ SCALAR_EAGER_UNARY(DayOfYear, "day_of_year")
834834
SCALAR_EAGER_UNARY(Hour, "hour")
835835
SCALAR_EAGER_UNARY(YearMonthDay, "year_month_day")
836836
SCALAR_EAGER_UNARY(IsDaylightSavings, "is_dst")
837-
SCALAR_EAGER_UNARY(LocalTime, "local_time")
837+
SCALAR_EAGER_UNARY(LocalTimestamp, "local_timestamp")
838838
SCALAR_EAGER_UNARY(IsLeapYear, "is_leap_year")
839839
SCALAR_EAGER_UNARY(ISOCalendar, "iso_calendar")
840840
SCALAR_EAGER_UNARY(ISOWeek, "iso_week")

cpp/src/arrow/compute/api_scalar.h

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1518,15 +1518,16 @@ ARROW_EXPORT Result<Datum> AssumeTimezone(const Datum& values,
15181518
ARROW_EXPORT Result<Datum> IsDaylightSavings(const Datum& values,
15191519
ExecContext* ctx = NULLPTR);
15201520

1521-
/// \brief LocalTime converts timestamp to timezone naive local timestamp
1521+
/// \brief LocalTimestamp converts timestamp to timezone naive local timestamp
15221522
///
15231523
/// \param[in] values input to convert to local time
15241524
/// \param[in] ctx the function execution context, optional
15251525
/// \return the resulting datum
15261526
///
15271527
/// \since 12.0.0
15281528
/// \note API not yet finalized
1529-
ARROW_EXPORT Result<Datum> LocalTime(const Datum& values, ExecContext* ctx = NULLPTR);
1529+
ARROW_EXPORT Result<Datum> LocalTimestamp(const Datum& values,
1530+
ExecContext* ctx = NULLPTR);
15301531

15311532
/// \brief Years Between finds the number of years between two values
15321533
///

cpp/src/arrow/compute/kernels/scalar_temporal_test.cc

Lines changed: 7 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -1811,7 +1811,7 @@ TEST_F(ScalarTemporalTest, TestTemporalDifferenceErrors) {
18111811
CallFunction("weeks_between", {arr1, arr1}, &options));
18121812
}
18131813

1814-
TEST_F(ScalarTemporalTest, TestLocalTime) {
1814+
TEST_F(ScalarTemporalTest, TestLocalTimestamp) {
18151815
const char* times_seconds_precision =
18161816
R"(["1970-01-01T00:00:59", "2000-02-29T23:23:23", "2033-05-18T03:33:20",
18171817
"2020-01-01T01:05:05", "2019-12-31T02:10:10", "2019-12-30T03:15:15",
@@ -1833,13 +1833,13 @@ TEST_F(ScalarTemporalTest, TestLocalTime) {
18331833
"2008-12-27 14:30:00", "2008-12-28 14:30:00", "2011-12-31 15:32:03", null])";
18341834

18351835
for (auto u : TimeUnit::values()) {
1836-
CheckScalarUnary("local_time", timestamp(u), times_seconds_precision, timestamp(u),
1837-
times_seconds_precision);
1838-
CheckScalarUnary("local_time", timestamp(u, "UTC"), times_seconds_precision,
1836+
CheckScalarUnary("local_timestamp", timestamp(u), times_seconds_precision,
18391837
timestamp(u), times_seconds_precision);
1840-
CheckScalarUnary("local_time", timestamp(u, "Asia/Kolkata"), times_seconds_precision,
1841-
timestamp(u), expected_local_kolkata);
1842-
CheckScalarUnary("local_time", timestamp(u, "Pacific/Marquesas"),
1838+
CheckScalarUnary("local_timestamp", timestamp(u, "UTC"), times_seconds_precision,
1839+
timestamp(u), times_seconds_precision);
1840+
CheckScalarUnary("local_timestamp", timestamp(u, "Asia/Kolkata"),
1841+
times_seconds_precision, timestamp(u), expected_local_kolkata);
1842+
CheckScalarUnary("local_timestamp", timestamp(u, "Pacific/Marquesas"),
18431843
times_seconds_precision, timestamp(u), expected_local_marquesas);
18441844
}
18451845
}

cpp/src/arrow/compute/kernels/scalar_temporal_unary.cc

Lines changed: 19 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -673,11 +673,11 @@ struct IsDaylightSavings {
673673
};
674674

675675
// ----------------------------------------------------------------------
676-
// Extract local time of a given timestamp given its timezone
676+
// Extract local timestamp of a given timestamp given its timezone
677677

678678
template <typename Duration, typename Localizer>
679-
struct LocalTime {
680-
explicit LocalTime(const FunctionOptions* options, Localizer&& localizer)
679+
struct LocalTimestamp {
680+
explicit LocalTimestamp(const FunctionOptions* options, Localizer&& localizer)
681681
: localizer_(std::move(localizer)) {}
682682

683683
template <typename T, typename Arg0>
@@ -1344,8 +1344,8 @@ Result<TypeHolder> ResolveAssumeTimezoneOutput(KernelContext* ctx,
13441344
return timestamp(in_type.unit(), AssumeTimezoneState::Get(ctx).timezone);
13451345
}
13461346

1347-
Result<TypeHolder> ResolveLocalTimeOutput(KernelContext* ctx,
1348-
const std::vector<TypeHolder>& args) {
1347+
Result<TypeHolder> ResolveLocalTimestampOutput(KernelContext* ctx,
1348+
const std::vector<TypeHolder>& args) {
13491349
const auto& in_type = checked_cast<const TimestampType&>(*args[0]);
13501350
return timestamp(in_type.unit());
13511351
}
@@ -1807,11 +1807,13 @@ const FunctionDoc is_dst_doc{
18071807
"An error is returned if the values do not have a defined timezone."),
18081808
{"values"}};
18091809

1810-
const FunctionDoc local_time_doc{
1810+
const FunctionDoc local_timestamp_doc{
18111811
"Convert timestamp to a timezone-naive local time timestamp",
1812-
("LocalTime converts a timestamp to a local time of timestamps timezone\n"
1813-
"and removes timezone metadata. If input is in UTC or doesn't have\n"
1814-
"timezone metadata, it is returned as is.\n"
1812+
("LocalTimestamp converts timezone-aware timestamp to local timestamp\n"
1813+
"of the given timestamp's timezone and removes timezone metadata.\n"
1814+
"Alternative name for this timestamp is also wall clock time.\n"
1815+
"If input is in UTC or without timezone, then unchanged input values\n"
1816+
"without timezone metadata are returned.\n"
18151817
"Null values emit null."),
18161818
{"values"}};
18171819
const FunctionDoc floor_temporal_doc{
@@ -1831,9 +1833,8 @@ const FunctionDoc ceil_temporal_doc{
18311833
const FunctionDoc round_temporal_doc{
18321834
"Round temporal values to the nearest multiple of specified time unit",
18331835
("Null values emit null.\n"
1834-
"If timezone is not given then timezone naive timestamp in UTC are\n"
1835-
"returned. An error is returned if the values have a defined timezone\n"
1836-
"but it cannot be found in the timezone database."),
1836+
"An error is returned if the values have a defined timezone but it\n"
1837+
"cannot be found in the timezone database."),
18371838
{"timestamps"},
18381839
"RoundTemporalOptions"};
18391840

@@ -2000,11 +2001,12 @@ void RegisterScalarTemporalUnary(FunctionRegistry* registry) {
20002001
is_dst_doc);
20012002
DCHECK_OK(registry->AddFunction(std::move(is_dst)));
20022003

2003-
auto local_time =
2004-
UnaryTemporalFactory<LocalTime, TemporalComponentExtract, TimestampType>::Make<
2005-
WithTimestamps>("local_time", OutputType::Resolver(ResolveLocalTimeOutput),
2006-
local_time_doc);
2007-
DCHECK_OK(registry->AddFunction(std::move(local_time)));
2004+
auto local_timestamp =
2005+
UnaryTemporalFactory<LocalTimestamp, TemporalComponentExtract, TimestampType>::Make<
2006+
WithTimestamps>("local_timestamp",
2007+
OutputType::Resolver(ResolveLocalTimestampOutput),
2008+
local_timestamp_doc);
2009+
DCHECK_OK(registry->AddFunction(std::move(local_timestamp)));
20082010

20092011
// Temporal rounding functions
20102012
// Note: UnaryTemporalFactory will not correctly resolve OutputType(FirstType) to

docs/source/cpp/compute.rst

Lines changed: 9 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -1556,20 +1556,20 @@ Input timestamps are assumed to be relative to the timezone given in
15561556
UTC-relative timestamps with the timezone metadata set to the above value.
15571557
An error is returned if the timestamps already have the timezone metadata set.
15581558

1559-
`local_time` function converts UTC-relative timestamps to local "timezone-naive"
1559+
`local_timestamp` function converts UTC-relative timestamps to local "timezone-naive"
15601560
timestamps. The timezone is taken from the timezone metadata of the input
15611561
timestamps. This function is the inverse of `assume_timezone`. Please note:
15621562
**all temporal functions already operate on timestamps as if they were in local
1563-
time of the metadata provided timezone**. Using `local_time` is only meant to be
1563+
time of the metadata provided timezone**. Using `local_timestamp` is only meant to be
15641564
used when an external system expects local timestamps.
15651565

1566-
+--------------------+------------+-------------------+---------------+----------------------------------+-------+
1567-
| Function name | Arity | Input types | Output type | Options class | Notes |
1568-
+====================+============+===================+===============+==================================+=======+
1569-
| assume_timezone | Unary | Timestamp | Timestamp | :struct:`AssumeTimezoneOptions` | \(1) |
1570-
+--------------------+------------+-------------------+---------------+----------------------------------+-------+
1571-
| local_time | Unary | Timestamp | Timestamp | | \(2) |
1572-
+--------------------+------------+-------------------+---------------+----------------------------------+-------+
1566+
+-----------------+-------+-------------+---------------+---------------------------------+-------+
1567+
| Function name | Arity | Input types | Output type | Options class | Notes |
1568+
+=================+=======+=============+===============+=================================+=======+
1569+
| assume_timezone | Unary | Timestamp | Timestamp | :struct:`AssumeTimezoneOptions` | \(1) |
1570+
+-----------------+-------+-------------+---------------+---------------------------------+-------+
1571+
| local_timestamp | Unary | Timestamp | Timestamp | | \(2) |
1572+
+-----------------+-------+-------------+---------------+---------------------------------+-------+
15731573

15741574
* \(1) In addition to the timezone value, :struct:`AssumeTimezoneOptions`
15751575
allows choosing the behaviour when a timestamp is ambiguous or nonexistent

python/pyarrow/tests/test_compute.py

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1951,7 +1951,7 @@ def _check_datetime_components(timestamps, timezone=None):
19511951
assert pc.microsecond(tsa).equals(pa.array(ts.dt.microsecond % 10 ** 3))
19521952
assert pc.nanosecond(tsa).equals(pa.array(ts.dt.nanosecond))
19531953
assert pc.subsecond(tsa).equals(pa.array(subseconds))
1954-
assert pc.local_time(tsa).equals(pa.array(ts.dt.tz_localize(None)))
1954+
assert pc.local_timestamp(tsa).equals(pa.array(ts.dt.tz_localize(None)))
19551955

19561956
if ts.dt.tz:
19571957
if ts.dt.tz is datetime.timezone.utc:

0 commit comments

Comments
 (0)