You are viewing a plain text version of this content. The canonical link for it is here.
Posted to github@arrow.apache.org by GitBox <gi...@apache.org> on 2022/07/04 14:19:04 UTC
[GitHub] [arrow] pitrou commented on a diff in pull request #13462: ARROW-16932: [C++] Rounding RoundTemporalOptions.calendar_based_origin doesn't correctly offset non-UTC results
pitrou commented on code in PR #13462:
URL: https://github.com/apache/arrow/pull/13462#discussion_r913040588
##########
cpp/src/arrow/compute/kernels/scalar_temporal_test.cc:
##########
@@ -2911,6 +3032,129 @@ TEST_F(ScalarTemporalTest, TestFloorTemporalMultipleSinceGreaterUnit) {
CheckScalarUnary(op, unit, times, unit, floor_15_years, &round_to_15_years);
}
+TEST_F(ScalarTemporalTest, TestFloorTemporalMultipleSinceGreaterUnitZoned) {
+ std::string op = "floor_temporal";
+ RoundTemporalOptions round_to_15_nanoseconds =
+ RoundTemporalOptions(15, CalendarUnit::NANOSECOND, true, true, true);
+ RoundTemporalOptions round_to_15_microseconds =
+ RoundTemporalOptions(15, CalendarUnit::MICROSECOND, true, true, true);
+ RoundTemporalOptions round_to_15_milliseconds =
+ RoundTemporalOptions(15, CalendarUnit::MILLISECOND, true, true, true);
+ RoundTemporalOptions round_to_13_seconds =
+ RoundTemporalOptions(13, CalendarUnit::SECOND, true, true, true);
+ RoundTemporalOptions round_to_13_minutes =
+ RoundTemporalOptions(13, CalendarUnit::MINUTE, true, true, true);
+ RoundTemporalOptions round_to_15_hours =
+ RoundTemporalOptions(15, CalendarUnit::HOUR, true, true, true);
+ RoundTemporalOptions round_to_15_days =
+ RoundTemporalOptions(15, CalendarUnit::DAY, true, true, true);
+ RoundTemporalOptions round_to_15_weeks =
+ RoundTemporalOptions(15, CalendarUnit::WEEK, true, true, true);
+ RoundTemporalOptions round_to_15_weeks_sunday =
+ RoundTemporalOptions(15, CalendarUnit::WEEK, false, true, true);
+ RoundTemporalOptions round_to_5_months =
+ RoundTemporalOptions(5, CalendarUnit::MONTH, true, true, true);
+ RoundTemporalOptions round_to_3_quarters =
+ RoundTemporalOptions(3, CalendarUnit::QUARTER, true, true, true);
+ RoundTemporalOptions round_to_15_years =
+ RoundTemporalOptions(15, CalendarUnit::YEAR, true, true, true);
+
+ // Data for tests below was generaed via lubridate with the exception
Review Comment:
```suggestion
// Data for tests below was generated via lubridate with the exception
```
##########
cpp/src/arrow/compute/kernels/scalar_temporal_test.cc:
##########
@@ -2593,6 +2593,127 @@ TEST_F(ScalarTemporalTest, TestCeilTemporalMultipleSinceGreaterUnit) {
CheckScalarUnary(op, unit, times, unit, ceil_15_years, &round_to_15_years);
}
+TEST_F(ScalarTemporalTest, TestCeilTemporalMultipleSinceGreaterUnitZoned) {
+ std::string op = "ceil_temporal";
+ RoundTemporalOptions round_to_15_nanoseconds =
+ RoundTemporalOptions(15, CalendarUnit::NANOSECOND, true, true, true);
+ RoundTemporalOptions round_to_15_microseconds =
+ RoundTemporalOptions(15, CalendarUnit::MICROSECOND, true, true, true);
+ RoundTemporalOptions round_to_15_milliseconds =
+ RoundTemporalOptions(15, CalendarUnit::MILLISECOND, true, true, true);
+ RoundTemporalOptions round_to_13_seconds =
+ RoundTemporalOptions(13, CalendarUnit::SECOND, true, true, true);
+ RoundTemporalOptions round_to_13_minutes =
+ RoundTemporalOptions(13, CalendarUnit::MINUTE, true, true, true);
+ RoundTemporalOptions round_to_15_hours =
+ RoundTemporalOptions(15, CalendarUnit::HOUR, true, true, true);
+ RoundTemporalOptions round_to_15_days =
+ RoundTemporalOptions(15, CalendarUnit::DAY, true, true, true);
+ RoundTemporalOptions round_to_15_weeks =
+ RoundTemporalOptions(15, CalendarUnit::WEEK, true, true, true);
+ RoundTemporalOptions round_to_15_weeks_sunday =
+ RoundTemporalOptions(15, CalendarUnit::WEEK, false, true, true);
+ RoundTemporalOptions round_to_5_months =
+ RoundTemporalOptions(5, CalendarUnit::MONTH, true, true, true);
+ RoundTemporalOptions round_to_3_quarters =
+ RoundTemporalOptions(3, CalendarUnit::QUARTER, true, true, true);
+ RoundTemporalOptions round_to_15_years =
+ RoundTemporalOptions(15, CalendarUnit::YEAR, true, true, true);
+
+ // Data for tests below was generaed via lubridate with the exception
+ // of week data because lubridate currently does not support rounding to
+ // multiple of week.
+ const char* ceil_15_nanosecond =
+ R"(["1970-01-01 00:00:59.123456795", "2000-02-29 23:23:24.000000005",
+ "1899-01-01 00:59:20.001001015", "2033-05-18 03:33:20.000000015",
+ "2020-01-01 01:05:05.001000015", "2019-12-31 02:10:10.002000015",
+ "2019-12-30 03:15:15.003000015", "2009-12-31 04:20:20.004132015",
+ "2010-01-01 05:25:25.005321015", "2010-01-03 06:30:30.006163015",
+ "2010-01-04 07:35:35.000000015", "2006-01-01 08:40:40.000000015",
+ "2005-12-31 09:45:45.000000015", "2008-12-28 00:00:00.000000015",
+ "2008-12-29 00:00:00.000000015", "2012-01-01 01:02:03.000000015", null])";
+ const char* ceil_15_microsecond =
+ R"(["1970-01-01 00:00:59.123465", "2000-02-29 23:23:24.000005",
+ "1899-01-01 00:59:20.001015", "2033-05-18 03:33:20.000015",
+ "2020-01-01 01:05:05.001015", "2019-12-31 02:10:10.002015",
+ "2019-12-30 03:15:15.003015", "2009-12-31 04:20:20.004135",
+ "2010-01-01 05:25:25.005330", "2010-01-03 06:30:30.006165",
+ "2010-01-04 07:35:35.000015", "2006-01-01 08:40:40.000015",
+ "2005-12-31 09:45:45.000015", "2008-12-28 00:00:00.000015",
+ "2008-12-29 00:00:00.000015", "2012-01-01 01:02:03.000015", null])";
+ const char* ceil_15_millisecond =
+ R"(["1970-01-01 00:00:59.135", "2000-02-29 23:23:24.005",
+ "1899-01-01 00:59:20.015", "2033-05-18 03:33:20.015",
+ "2020-01-01 01:05:05.015", "2019-12-31 02:10:10.015",
+ "2019-12-30 03:15:15.015", "2009-12-31 04:20:20.015",
+ "2010-01-01 05:25:25.015", "2010-01-03 06:30:30.015",
+ "2010-01-04 07:35:35.015", "2006-01-01 08:40:40.015",
+ "2005-12-31 09:45:45.015", "2008-12-28 00:00:00.015",
+ "2008-12-29 00:00:00.015", "2012-01-01 01:02:03.015", null])";
+ const char* ceil_13_second = R"([
+ "1970-01-01 00:01:05", "2000-02-29 23:23:26", "1899-01-01 00:59:26", "2033-05-18 03:33:26",
+ "2020-01-01 01:05:13", "2019-12-31 02:10:13", "2019-12-30 03:15:26", "2009-12-31 04:20:26",
+ "2010-01-01 05:25:26", "2010-01-03 06:30:39", "2010-01-04 07:35:39", "2006-01-01 08:40:52",
+ "2005-12-31 09:45:52", "2008-12-28 00:00:13", "2008-12-29 00:00:13", "2012-01-01 01:02:13", null])";
+ const char* ceil_13_minute = R"([
+ "1970-01-01 00:09:00", "2000-02-29 23:35:00", "1899-01-01 01:05:00", "2033-05-18 03:43:00",
+ "2020-01-01 01:09:00", "2019-12-31 02:22:00", "2019-12-30 03:22:00", "2009-12-31 04:22:00",
+ "2010-01-01 05:35:00", "2010-01-03 06:43:00", "2010-01-04 07:43:00", "2006-01-01 08:43:00",
+ "2005-12-31 09:56:00", "2008-12-28 00:09:00", "2008-12-29 00:09:00", "2012-01-01 01:09:00", null])";
+ const char* ceil_15_hour = R"([
+ "1970-01-01 05:30:00", "2000-03-01 04:30:00", "1899-01-01 06:00:00", "2033-05-18 05:30:00",
+ "2020-01-01 04:30:00", "2019-12-31 04:30:00", "2019-12-30 04:30:00", "2009-12-31 04:30:00",
+ "2010-01-01 19:30:00", "2010-01-03 19:30:00", "2010-01-04 19:30:00", "2006-01-01 19:30:00",
+ "2005-12-31 19:30:00", "2008-12-28 04:30:00", "2008-12-29 04:30:00", "2012-01-01 04:30:00", null])";
+ const char* ceil_15_day = R"([
+ "1970-01-15 14:30:00", "2000-03-15 13:30:00", "1899-01-15 15:00:00", "2033-05-30 14:30:00",
+ "2020-01-15 13:30:00", "2020-01-14 13:30:00", "2019-12-30 13:30:00", "2010-01-14 13:30:00",
+ "2010-01-15 13:30:00", "2010-01-15 13:30:00", "2010-01-15 13:30:00", "2006-01-15 13:30:00",
+ "2006-01-14 13:30:00", "2008-12-30 13:30:00", "2008-12-30 13:30:00", "2012-01-15 13:30:00", null])";
+ const char* ceil_15_weeks = R"([
+ "1970-04-12 14:30:00", "2000-04-16 14:30:00", "1899-04-16 15:00:00", "2033-07-31 14:30:00",
+ "2020-04-12 14:30:00", "2020-04-12 14:30:00", "2020-04-12 14:30:00", "2010-04-18 14:30:00",
+ "2010-04-18 14:30:00", "2010-04-18 14:30:00", "2010-04-18 14:30:00", "2006-04-16 14:30:00",
+ "2006-04-16 14:30:00", "2009-02-22 13:30:00", "2009-04-12 14:30:00", "2012-04-15 14:30:00", null])";
+ const char* ceil_15_weeks_sunday = R"([
+ "1970-04-18 14:30:00", "2000-04-15 14:30:00", "1899-04-15 15:00:00", "2033-07-30 14:30:00",
+ "2020-04-11 14:30:00", "2020-04-11 14:30:00", "2020-04-11 14:30:00", "2010-04-17 14:30:00",
+ "2010-04-17 14:30:00", "2010-04-17 14:30:00", "2010-04-17 14:30:00", "2006-04-15 14:30:00",
+ "2006-04-15 14:30:00", "2009-04-18 14:30:00", "2009-04-18 14:30:00", "2012-04-14 14:30:00", null])";
+ const char* ceil_5_months = R"([
+ "1970-05-31 14:30:00", "2000-05-31 14:30:00", "1899-05-31 14:30:00", "2033-05-31 14:30:00",
+ "2020-05-31 14:30:00", "2020-03-31 13:30:00", "2020-03-31 13:30:00", "2010-03-31 13:30:00",
+ "2010-05-31 14:30:00", "2010-05-31 14:30:00", "2010-05-31 14:30:00", "2006-05-31 14:30:00",
+ "2006-03-31 13:30:00", "2009-03-31 13:30:00", "2009-03-31 13:30:00", "2012-05-31 14:30:00", null])";
+ const char* ceil_3_quarters = R"([
+ "1970-09-30 14:30:00", "2000-09-30 14:30:00", "1899-09-30 14:30:00", "2033-09-30 14:30:00",
+ "2020-09-30 14:30:00", "2020-06-30 14:30:00", "2020-06-30 14:30:00", "2010-06-30 14:30:00",
+ "2010-09-30 14:30:00", "2010-09-30 14:30:00", "2010-09-30 14:30:00", "2006-09-30 14:30:00",
+ "2006-06-30 14:30:00", "2009-06-30 14:30:00", "2009-06-30 14:30:00", "2012-09-30 14:30:00", null])";
+ const char* ceil_15_years = R"([
+ "1979-12-31 13:30:00", "2009-12-31 13:30:00", "1904-12-31 14:30:00", "2039-12-31 13:30:00",
+ "2024-12-31 13:30:00", "2024-12-31 13:30:00", "2024-12-31 13:30:00", "2009-12-31 13:30:00",
+ "2024-12-31 13:30:00", "2024-12-31 13:30:00", "2024-12-31 13:30:00", "2009-12-31 13:30:00",
+ "2009-12-31 13:30:00", "2009-12-31 13:30:00", "2009-12-31 13:30:00", "2024-12-31 13:30:00", null])";
+
+ auto unit = timestamp(TimeUnit::NANO, "Australia/Broken_Hill");
Review Comment:
Can you comment what the concrete offsets and DST times are?
##########
cpp/src/arrow/compute/kernels/scalar_temporal_test.cc:
##########
@@ -2593,6 +2593,127 @@ TEST_F(ScalarTemporalTest, TestCeilTemporalMultipleSinceGreaterUnit) {
CheckScalarUnary(op, unit, times, unit, ceil_15_years, &round_to_15_years);
}
+TEST_F(ScalarTemporalTest, TestCeilTemporalMultipleSinceGreaterUnitZoned) {
+ std::string op = "ceil_temporal";
+ RoundTemporalOptions round_to_15_nanoseconds =
Review Comment:
Are these options repeated around? Perhaps they should be factored in a test fixture class?
##########
cpp/src/arrow/compute/kernels/scalar_temporal_test.cc:
##########
@@ -2593,6 +2593,127 @@ TEST_F(ScalarTemporalTest, TestCeilTemporalMultipleSinceGreaterUnit) {
CheckScalarUnary(op, unit, times, unit, ceil_15_years, &round_to_15_years);
}
+TEST_F(ScalarTemporalTest, TestCeilTemporalMultipleSinceGreaterUnitZoned) {
+ std::string op = "ceil_temporal";
+ RoundTemporalOptions round_to_15_nanoseconds =
+ RoundTemporalOptions(15, CalendarUnit::NANOSECOND, true, true, true);
+ RoundTemporalOptions round_to_15_microseconds =
+ RoundTemporalOptions(15, CalendarUnit::MICROSECOND, true, true, true);
+ RoundTemporalOptions round_to_15_milliseconds =
+ RoundTemporalOptions(15, CalendarUnit::MILLISECOND, true, true, true);
+ RoundTemporalOptions round_to_13_seconds =
+ RoundTemporalOptions(13, CalendarUnit::SECOND, true, true, true);
+ RoundTemporalOptions round_to_13_minutes =
+ RoundTemporalOptions(13, CalendarUnit::MINUTE, true, true, true);
+ RoundTemporalOptions round_to_15_hours =
+ RoundTemporalOptions(15, CalendarUnit::HOUR, true, true, true);
+ RoundTemporalOptions round_to_15_days =
+ RoundTemporalOptions(15, CalendarUnit::DAY, true, true, true);
+ RoundTemporalOptions round_to_15_weeks =
+ RoundTemporalOptions(15, CalendarUnit::WEEK, true, true, true);
+ RoundTemporalOptions round_to_15_weeks_sunday =
+ RoundTemporalOptions(15, CalendarUnit::WEEK, false, true, true);
+ RoundTemporalOptions round_to_5_months =
+ RoundTemporalOptions(5, CalendarUnit::MONTH, true, true, true);
+ RoundTemporalOptions round_to_3_quarters =
+ RoundTemporalOptions(3, CalendarUnit::QUARTER, true, true, true);
+ RoundTemporalOptions round_to_15_years =
+ RoundTemporalOptions(15, CalendarUnit::YEAR, true, true, true);
+
+ // Data for tests below was generaed via lubridate with the exception
Review Comment:
```suggestion
// Data for tests below was generated via lubridate with the exception
```
##########
cpp/src/arrow/compute/kernels/scalar_temporal_test.cc:
##########
@@ -3249,6 +3493,128 @@ TEST_F(ScalarTemporalTest, TestRoundTemporalMultipleSinceGreaterUnit) {
CheckScalarUnary(op, unit, times, unit, round_15_years, &round_to_15_years);
}
+TEST_F(ScalarTemporalTest, TestRoundTemporalMultipleSinceGreaterUnitZoned) {
+ std::string op = "round_temporal";
+ RoundTemporalOptions round_to_15_nanoseconds =
+ RoundTemporalOptions(15, CalendarUnit::NANOSECOND, true, true, true);
+ RoundTemporalOptions round_to_15_microseconds =
+ RoundTemporalOptions(15, CalendarUnit::MICROSECOND, true, true, true);
+ RoundTemporalOptions round_to_15_milliseconds =
+ RoundTemporalOptions(15, CalendarUnit::MILLISECOND, true, true, true);
+ RoundTemporalOptions round_to_13_seconds =
+ RoundTemporalOptions(13, CalendarUnit::SECOND, true, true, true);
+ RoundTemporalOptions round_to_13_minutes =
+ RoundTemporalOptions(13, CalendarUnit::MINUTE, true, true, true);
+ RoundTemporalOptions round_to_15_hours =
+ RoundTemporalOptions(15, CalendarUnit::HOUR, true, true, true);
+ RoundTemporalOptions round_to_15_days =
+ RoundTemporalOptions(15, CalendarUnit::DAY, true, true, true);
+ RoundTemporalOptions round_to_15_weeks =
+ RoundTemporalOptions(15, CalendarUnit::WEEK, true, true, true);
+ RoundTemporalOptions round_to_15_weeks_sunday =
+ RoundTemporalOptions(15, CalendarUnit::WEEK, false, true, true);
+ RoundTemporalOptions round_to_5_months =
+ RoundTemporalOptions(5, CalendarUnit::MONTH, true, true, true);
+ RoundTemporalOptions round_to_3_quarters =
+ RoundTemporalOptions(3, CalendarUnit::QUARTER, true, true, true);
+ RoundTemporalOptions round_to_15_years =
+ RoundTemporalOptions(15, CalendarUnit::YEAR, true, true, true);
+
+ // Data for tests below was generaed via lubridate with the exception
Review Comment:
```suggestion
// Data for tests below was generated via lubridate with the exception
```
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org