GH-15053: [C++] Add option to string 'center' kernel to control left/right alignment on odd number of padding (#41449)

jorisvandenbossche · web-flow · commit 5d58fc68c6d6 · 2024-06-26T17:35:52.000+02:00
### Rationale for this change See the issue #15053 for some more context, but in summary: for the "center" padding, and the number of characters that are being added, one needs to decide whether to add one more character on the left or right. Our implementation (somewhat randomly, I think) decided to put the extra space on the right. The Python standard library however, puts the extra space on the left. And for the usage of pyarrow as a string compute engine in the pandas project, we would like to have the option to have consistent behaviour with Python. ### What changes are included in this PR? Add an option `align_left_on_odd_padding` to `PadOptions` that controls where the extra space is put. This keyword is quite ugly, but I am not sure what other solution there is if we want to give pyarrow users this option (also happy to hear other argument name options) ### Are these changes tested? Yes ### Are there any user-facing changes? No * GitHub Issue: #15053 Authored-by: Joris Van den Bossche <jorisvandenbossche@gmail.com> Signed-off-by: Joris Van den Bossche <jorisvandenbossche@gmail.com>
diff --git a/cpp/src/arrow/compute/api_scalar.cc b/cpp/src/arrow/compute/api_scalar.cc
@@ -341,7 +341,8 @@ static auto kMatchSubstringOptionsType = GetFunctionOptionsType<MatchSubstringOp
 static auto kNullOptionsType = GetFunctionOptionsType<NullOptions>(
     DataMember("nan_is_null", &NullOptions::nan_is_null));
 static auto kPadOptionsType = GetFunctionOptionsType<PadOptions>(
-    DataMember("width", &PadOptions::width), DataMember("padding", &PadOptions::padding));
+    DataMember("width", &PadOptions::width), DataMember("padding", &PadOptions::padding),
+    DataMember("lean_left_on_odd_padding", &PadOptions::lean_left_on_odd_padding));
 static auto kReplaceSliceOptionsType = GetFunctionOptionsType<ReplaceSliceOptions>(
     DataMember("start", &ReplaceSliceOptions::start),
     DataMember("stop", &ReplaceSliceOptions::stop),
@@ -480,10 +481,11 @@ NullOptions::NullOptions(bool nan_is_null)
     : FunctionOptions(internal::kNullOptionsType), nan_is_null(nan_is_null) {}
 constexpr char NullOptions::kTypeName[];
 
-PadOptions::PadOptions(int64_t width, std::string padding)
+PadOptions::PadOptions(int64_t width, std::string padding, bool lean_left_on_odd_padding)
     : FunctionOptions(internal::kPadOptionsType),
       width(width),
-      padding(std::move(padding)) {}
+      padding(std::move(padding)),
+      lean_left_on_odd_padding(lean_left_on_odd_padding) {}
 PadOptions::PadOptions() : PadOptions(0, " ") {}
 constexpr char PadOptions::kTypeName[];
 
diff --git a/cpp/src/arrow/compute/api_scalar.h b/cpp/src/arrow/compute/api_scalar.h
@@ -358,14 +358,19 @@ class ARROW_EXPORT StrftimeOptions : public FunctionOptions {
 
 class ARROW_EXPORT PadOptions : public FunctionOptions {
  public:
-  explicit PadOptions(int64_t width, std::string padding = " ");
+  explicit PadOptions(int64_t width, std::string padding = " ",
+                      bool lean_left_on_odd_padding = true);
   PadOptions();
   static constexpr char const kTypeName[] = "PadOptions";
 
   /// The desired string length.
   int64_t width;
   /// What to pad the string with. Should be one codepoint (Unicode)/byte (ASCII).
   std::string padding;
+  /// What to do if there is an odd number of padding characters (in case of centered
+  /// padding). Defaults to aligning on the left (i.e. adding the extra padding character
+  /// on the right)
+  bool lean_left_on_odd_padding = true;
 };
 
 class ARROW_EXPORT TrimOptions : public FunctionOptions {
diff --git a/cpp/src/arrow/compute/function_test.cc b/cpp/src/arrow/compute/function_test.cc
@@ -102,6 +102,7 @@ TEST(FunctionOptions, Equality) {
 #endif
   options.emplace_back(new PadOptions(5, " "));
   options.emplace_back(new PadOptions(10, "A"));
+  options.emplace_back(new PadOptions(10, "A", false));
   options.emplace_back(new TrimOptions(" "));
   options.emplace_back(new TrimOptions("abc"));
   options.emplace_back(new SliceOptions(/*start=*/1));
diff --git a/cpp/src/arrow/compute/kernels/scalar_string_ascii.cc b/cpp/src/arrow/compute/kernels/scalar_string_ascii.cc
@@ -1142,9 +1142,13 @@ struct AsciiPadTransform : public StringTransformBase {
     int64_t left = 0;
     int64_t right = 0;
     if (PadLeft && PadRight) {
-      // If odd number of spaces, put the extra space on the right
-      left = spaces / 2;
-      right = spaces - left;
+      if (options_.lean_left_on_odd_padding) {
+        left = spaces / 2;
+        right = spaces - left;
+      } else {
+        right = spaces / 2;
+        left = spaces - right;
+      }
     } else if (PadLeft) {
       left = spaces;
     } else if (PadRight) {
diff --git a/cpp/src/arrow/compute/kernels/scalar_string_test.cc b/cpp/src/arrow/compute/kernels/scalar_string_test.cc
@@ -2117,6 +2117,12 @@ TYPED_TEST(TestStringKernels, PadUTF8) {
       R"([null, "a\u2008\u2008\u2008\u2008", "bb\u2008\u2008\u2008", "b\u00E1r\u2008\u2008", "foobar"])",
       &options);
 
+  PadOptions options2{/*width=*/5, "\xe2\x80\x88", /*lean_left_on_odd_padding=*/false};
+  this->CheckUnary(
+      "utf8_center", R"([null, "a", "bb", "b\u00E1r", "foobar"])", this->type(),
+      R"([null, "\u2008\u2008a\u2008\u2008", "\u2008\u2008bb\u2008", "\u2008b\u00E1r\u2008", "foobar"])",
+      &options2);
+
   PadOptions options_bad{/*width=*/3, /*padding=*/"spam"};
   auto input = ArrayFromJSON(this->type(), R"(["foo"])");
   EXPECT_RAISES_WITH_MESSAGE_THAT(Invalid,
@@ -2459,6 +2465,10 @@ TYPED_TEST(TestStringKernels, PadAscii) {
   this->CheckUnary("ascii_rpad", R"([null, "a", "bb", "bar", "foobar"])", this->type(),
                    R"([null, "a    ", "bb   ", "bar  ", "foobar"])", &options);
 
+  PadOptions options2{/*width=*/5, " ", /*lean_left_on_odd_padding=*/false};
+  this->CheckUnary("ascii_center", R"([null, "a", "bb", "bar", "foobar"])", this->type(),
+                   R"([null, "  a  ", "  bb ", " bar ", "foobar"])", &options2);
+
   PadOptions options_bad{/*width=*/3, /*padding=*/"spam"};
   auto input = ArrayFromJSON(this->type(), R"(["foo"])");
   EXPECT_RAISES_WITH_MESSAGE_THAT(Invalid,
diff --git a/cpp/src/arrow/compute/kernels/scalar_string_utf8.cc b/cpp/src/arrow/compute/kernels/scalar_string_utf8.cc
@@ -930,9 +930,13 @@ struct Utf8PadTransform : public StringTransformBase {
     int64_t left = 0;
     int64_t right = 0;
     if (PadLeft && PadRight) {
-      // If odd number of spaces, put the extra space on the right
-      left = spaces / 2;
-      right = spaces - left;
+      if (options_.lean_left_on_odd_padding) {
+        left = spaces / 2;
+        right = spaces - left;
+      } else {
+        right = spaces / 2;
+        left = spaces - right;
+      }
     } else if (PadLeft) {
       left = spaces;
     } else if (PadRight) {
diff --git a/python/pyarrow/_compute.pyx b/python/pyarrow/_compute.pyx
@@ -1108,8 +1108,8 @@ class MatchSubstringOptions(_MatchSubstringOptions):
 
 
 cdef class _PadOptions(FunctionOptions):
-    def _set_options(self, width, padding):
-        self.wrapped.reset(new CPadOptions(width, tobytes(padding)))
+    def _set_options(self, width, padding, lean_left_on_odd_padding):
+        self.wrapped.reset(new CPadOptions(width, tobytes(padding), lean_left_on_odd_padding))
 
 
 class PadOptions(_PadOptions):
@@ -1122,10 +1122,14 @@ class PadOptions(_PadOptions):
         Desired string length.
     padding : str, default " "
         What to pad the string with. Should be one byte or codepoint.
+    lean_left_on_odd_padding : bool, default True
+        What to do if there is an odd number of padding characters (in case
+        of centered padding). Defaults to aligning on the left (i.e. adding
+        the extra padding character on the right).
     """
 
-    def __init__(self, width, padding=' '):
-        self._set_options(width, padding)
+    def __init__(self, width, padding=' ', lean_left_on_odd_padding=True):
+        self._set_options(width, padding, lean_left_on_odd_padding)
 
 
 cdef class _TrimOptions(FunctionOptions):
diff --git a/python/pyarrow/includes/libarrow.pxd b/python/pyarrow/includes/libarrow.pxd
@@ -2359,9 +2359,10 @@ cdef extern from "arrow/compute/api.h" namespace "arrow::compute" nogil:
 
     cdef cppclass CPadOptions \
             "arrow::compute::PadOptions"(CFunctionOptions):
-        CPadOptions(int64_t width, c_string padding)
+        CPadOptions(int64_t width, c_string padding, c_bool lean_left_on_odd_padding)
         int64_t width
         c_string padding
+        c_bool lean_left_on_odd_padding
 
     cdef cppclass CSliceOptions \
             "arrow::compute::SliceOptions"(CFunctionOptions):
