jorisvandenbossche commented on code in PR #39851:
URL: https://github.com/apache/arrow/pull/39851#discussion_r1504284247
##########
docs/source/cpp/compute.rst:
##########
@@ -1816,25 +1816,25 @@ Structural transforms
The output type is an Array of items for the ``FIRST``/``LAST`` options
and an Array of List of items for the ``ALL`` option.
-* \(6) Extract a child value based on a sequence of indices passed in
+* \(6) Extract a child value based on a field name or a sequence of indices
passed in
the options. The validity bitmap of the result will be the
intersection of all intermediate validity bitmaps. For example, for
an array with type ``struct<a: int32, b: struct<c: int64, d:
float64>>``:
- * An empty sequence of indices yields the original value unchanged.
- * The index ``0`` yields an array of type ``int32`` whose validity
+ * An empty sequence of indices yields the original value unchanged and in
case of an empty field name it yields an error.
+ * The index ``0`` or the field name ``a`` yields an array of type ``int32``
whose validity
bitmap is the intersection of the bitmap for the outermost struct
and the bitmap for the child ``a``.
- * The index ``1, 1`` yields an array of type ``float64`` whose
+ * The index ``1, 1`` or the field name ``b.d`` yields an array of type
``float64`` whose
Review Comment:
```suggestion
* The index ``1, 1`` or the field name ``"b.d"`` yields an array of type
``float64`` whose
```
##########
docs/source/cpp/compute.rst:
##########
@@ -1816,25 +1816,25 @@ Structural transforms
The output type is an Array of items for the ``FIRST``/``LAST`` options
and an Array of List of items for the ``ALL`` option.
-* \(6) Extract a child value based on a sequence of indices passed in
+* \(6) Extract a child value based on a field name or a sequence of indices
passed in
Review Comment:
Thinking about this once more, I realize this are the C++ docs, and at that
level, the argument is actually a FieldRef, which can be defined by a sequence
of indices or string names. The dotted path as how you added in the examples
below still needs to be converted to a FieldRef manually
(`FieldRef::FromDotPath`).
So maybe we should generalize this to "based on a field reference passed in
the options (this field reference can be specified as an index or name. or a
sequence thereof)" ?
##########
docs/source/cpp/compute.rst:
##########
@@ -1816,25 +1816,25 @@ Structural transforms
The output type is an Array of items for the ``FIRST``/``LAST`` options
and an Array of List of items for the ``ALL`` option.
-* \(6) Extract a child value based on a sequence of indices passed in
+* \(6) Extract a child value based on a field name or a sequence of indices
passed in
the options. The validity bitmap of the result will be the
intersection of all intermediate validity bitmaps. For example, for
an array with type ``struct<a: int32, b: struct<c: int64, d:
float64>>``:
- * An empty sequence of indices yields the original value unchanged.
- * The index ``0`` yields an array of type ``int32`` whose validity
+ * An empty sequence of indices yields the original value unchanged and in
case of an empty field name it yields an error.
+ * The index ``0`` or the field name ``a`` yields an array of type ``int32``
whose validity
Review Comment:
```suggestion
* The index ``0`` or the field name ``"a"`` yields an array of type
``int32`` whose validity
```
##########
docs/source/cpp/compute.rst:
##########
@@ -1816,25 +1816,25 @@ Structural transforms
The output type is an Array of items for the ``FIRST``/``LAST`` options
and an Array of List of items for the ``ALL`` option.
-* \(6) Extract a child value based on a sequence of indices passed in
+* \(6) Extract a child value based on a field name or a sequence of indices
passed in
the options. The validity bitmap of the result will be the
intersection of all intermediate validity bitmaps. For example, for
an array with type ``struct<a: int32, b: struct<c: int64, d:
float64>>``:
- * An empty sequence of indices yields the original value unchanged.
- * The index ``0`` yields an array of type ``int32`` whose validity
+ * An empty sequence of indices yields the original value unchanged and in
case of an empty field name it yields an error.
Review Comment:
```suggestion
* An empty sequence of indices yields the original value unchanged.
```
I would leave out this part as it's potentially just more confusing (what's
an empty name?)
##########
docs/source/cpp/compute.rst:
##########
@@ -1816,25 +1816,25 @@ Structural transforms
The output type is an Array of items for the ``FIRST``/``LAST`` options
and an Array of List of items for the ``ALL`` option.
-* \(6) Extract a child value based on a sequence of indices passed in
+* \(6) Extract a child value based on a field name or a sequence of indices
passed in
the options. The validity bitmap of the result will be the
intersection of all intermediate validity bitmaps. For example, for
an array with type ``struct<a: int32, b: struct<c: int64, d:
float64>>``:
- * An empty sequence of indices yields the original value unchanged.
- * The index ``0`` yields an array of type ``int32`` whose validity
+ * An empty sequence of indices yields the original value unchanged and in
case of an empty field name it yields an error.
+ * The index ``0`` or the field name ``a`` yields an array of type ``int32``
whose validity
bitmap is the intersection of the bitmap for the outermost struct
and the bitmap for the child ``a``.
- * The index ``1, 1`` yields an array of type ``float64`` whose
+ * The index ``1, 1`` or the field name ``b.d`` yields an array of type
``float64`` whose
validity bitmap is the intersection of the bitmaps for the
- outermost struct, for struct ``b``, and for the child ``d``.
+ outermost struct, for struct ``b`` and for the child ``d``.
Review Comment:
```suggestion
outermost struct, for struct ``b``, and for the child ``d``.
```
(this comma, while not strictly necessary, is actually correct, it's a
serial or oxford comma)
##########
docs/source/cpp/compute.rst:
##########
@@ -1816,25 +1816,25 @@ Structural transforms
The output type is an Array of items for the ``FIRST``/``LAST`` options
and an Array of List of items for the ``ALL`` option.
-* \(6) Extract a child value based on a sequence of indices passed in
+* \(6) Extract a child value based on a field name or a sequence of indices
passed in
the options. The validity bitmap of the result will be the
intersection of all intermediate validity bitmaps. For example, for
an array with type ``struct<a: int32, b: struct<c: int64, d:
float64>>``:
- * An empty sequence of indices yields the original value unchanged.
- * The index ``0`` yields an array of type ``int32`` whose validity
+ * An empty sequence of indices yields the original value unchanged and in
case of an empty field name it yields an error.
+ * The index ``0`` or the field name ``a`` yields an array of type ``int32``
whose validity
bitmap is the intersection of the bitmap for the outermost struct
and the bitmap for the child ``a``.
- * The index ``1, 1`` yields an array of type ``float64`` whose
+ * The index ``1, 1`` or the field name ``b.d`` yields an array of type
``float64`` whose
validity bitmap is the intersection of the bitmaps for the
- outermost struct, for struct ``b``, and for the child ``d``.
+ outermost struct, for struct ``b`` and for the child ``d``.
For unions, a validity bitmap is synthesized based on the type
codes. Also, the index is always the child index and not a type code.
Hence for array with type ``sparse_union<2: int32, 7: utf8>``:
- * The index ``0`` yields an array of type ``int32``, which is valid
+ * The index ``0`` or the field name ``2`` yields an array of type ``int32``,
which is valid
Review Comment:
```suggestion
* The index ``0`` or the field name ``"2"`` yields an array of type
``int32``, which is valid
```
--
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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
