You are viewing a plain text version of this content. The canonical link for it is here.
Posted to github@arrow.apache.org by "judahrand (via GitHub)" <gi...@apache.org> on 2023/10/30 22:54:40 UTC

[PR] Add `Table.to/from_struct_array` [arrow]

judahrand opened a new pull request, #38520:
URL: https://github.com/apache/arrow/pull/38520

   <!--
   Thanks for opening a pull request!
   If this is your first pull request you can find detailed information on how 
   to contribute here:
     * [New Contributor's Guide](https://arrow.apache.org/docs/dev/developers/guide/step_by_step/pr_lifecycle.html#reviews-and-merge-of-the-pull-request)
     * [Contributing Overview](https://arrow.apache.org/docs/dev/developers/overview.html)
   
   
   If this is not a [minor PR](https://github.com/apache/arrow/blob/main/CONTRIBUTING.md#Minor-Fixes). Could you open an issue for this pull request on GitHub? https://github.com/apache/arrow/issues/new/choose
   
   Opening GitHub issues ahead of time contributes to the [Openness](http://theapacheway.com/open/#:~:text=Openness%20allows%20new%20users%20the,must%20happen%20in%20the%20open.) of the Apache Arrow project.
   
   Then could you also rename the pull request title in the following format?
   
       GH-${GITHUB_ISSUE_ID}: [${COMPONENT}] ${SUMMARY}
   
   or
   
       MINOR: [${COMPONENT}] ${SUMMARY}
   
   In the case of PARQUET issues on JIRA the title also supports:
   
       PARQUET-${JIRA_ISSUE_ID}: [${COMPONENT}] ${SUMMARY}
   
   -->
   
   ### Rationale for this change
   
   <!--
    Why are you proposing this change? If this is already explained clearly in the issue then this section is not needed.
    Explaining clearly why changes are proposed helps reviewers understand your changes and offer better suggestions for fixes.  
   -->
   
   ### What changes are included in this PR?
   
   <!--
   There is no need to duplicate the description in the issue here but it is sometimes worth providing a summary of the individual changes in this PR.
   -->
   
   ### Are these changes tested?
   
   <!--
   We typically require tests for all PRs in order to:
   1. Prevent the code from being accidentally broken by subsequent changes
   2. Serve as another way to document the expected behavior of the code
   
   If tests are not included in your PR, please explain why (for example, are they covered by existing tests)?
   -->
   
   ### Are there any user-facing changes?
   
   <!--
   If there are user-facing changes then we may require documentation to be updated before approving the PR.
   -->
   
   <!--
   If there are any breaking changes to public APIs, please uncomment the line below and explain which changes are breaking.
   -->
   <!-- **This PR includes breaking changes to public APIs.** -->
   
   <!--
   Please uncomment the line below (and provide explanation) if the changes fix either (a) a security vulnerability, (b) a bug that caused incorrect or invalid data to be produced, or (c) a bug that causes a crash (even when the API contract is upheld). We use this to highlight fixes to issues that may affect users without their knowledge. For this reason, fixing bugs that cause errors don't count, since those are usually obvious.
   -->
   <!-- **This PR contains a "Critical Fix".** -->


-- 
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


Re: [PR] GH-33500 [Python] add `Table.to/from_struct_array` [arrow]

Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on PR #38520:
URL: https://github.com/apache/arrow/pull/38520#issuecomment-1786204484

   :warning: GitHub issue #33500 **has been automatically assigned in GitHub** to PR creator.


-- 
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


Re: [PR] Add `Table.to/from_struct_array` [arrow]

Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on PR #38520:
URL: https://github.com/apache/arrow/pull/38520#issuecomment-1786172215

   <!--
     Licensed to the Apache Software Foundation (ASF) under one
     or more contributor license agreements.  See the NOTICE file
     distributed with this work for additional information
     regarding copyright ownership.  The ASF licenses this file
     to you under the Apache License, Version 2.0 (the
     "License"); you may not use this file except in compliance
     with the License.  You may obtain a copy of the License at
   
       http://www.apache.org/licenses/LICENSE-2.0
   
     Unless required by applicable law or agreed to in writing,
     software distributed under the License is distributed on an
     "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
     KIND, either express or implied.  See the License for the
     specific language governing permissions and limitations
     under the License.
   -->
   
   Thanks for opening a pull request!
   
   If this is not a [minor PR](https://github.com/apache/arrow/blob/main/CONTRIBUTING.md#Minor-Fixes). Could you open an issue for this pull request on GitHub? https://github.com/apache/arrow/issues/new/choose
   
   Opening GitHub issues ahead of time contributes to the [Openness](http://theapacheway.com/open/#:~:text=Openness%20allows%20new%20users%20the,must%20happen%20in%20the%20open.) of the Apache Arrow project.
   
   Then could you also rename the pull request title in the following format?
   
       GH-${GITHUB_ISSUE_ID}: [${COMPONENT}] ${SUMMARY}
   
   or
   
       MINOR: [${COMPONENT}] ${SUMMARY}
   
   In the case of PARQUET issues on JIRA the title also supports:
   
       PARQUET-${JIRA_ISSUE_ID}: [${COMPONENT}] ${SUMMARY}
   
   See also:
   
     * [Other pull requests](https://github.com/apache/arrow/pulls/)
     * [Contribution Guidelines - How to contribute patches](https://arrow.apache.org/docs/developers/contributing.html#how-to-contribute-patches)
   


-- 
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


Re: [PR] GH-33500: [Python] add `Table.to/from_struct_array` [arrow]

Posted by "jorisvandenbossche (via GitHub)" <gi...@apache.org>.
jorisvandenbossche commented on PR #38520:
URL: https://github.com/apache/arrow/pull/38520#issuecomment-1881380465

   Thanks @judahrand!


-- 
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


Re: [PR] GH-33500: [Python] add `Table.to/from_struct_array` [arrow]

Posted by "kou (via GitHub)" <gi...@apache.org>.
kou commented on code in PR #38520:
URL: https://github.com/apache/arrow/pull/38520#discussion_r1423119887


##########
python/pyarrow/tests/test_table.py:
##########
@@ -878,6 +878,85 @@ def test_recordbatch_from_struct_array():
     ))
 
 
+def test_recordbatch_to_struct_array():
+    batch = pa.RecordBatch.from_arrays(
+        [
+            pa.array([1, None], type=pa.int32()),
+            pa.array([None, 1.0], type=pa.float32()),
+        ], ["ints", "floats"]
+    )
+    result = batch.to_struct_array()
+    assert result.equals(pa.array(
+        [{"ints": 1}, {"floats": 1.0}],
+        type=pa.struct([("ints", pa.int32()), ("floats", pa.float32())]),
+    ))
+
+
+def test_table_from_struct_array_invalid():
+    with pytest.raises(
+        TypeError,
+        match="Argument 'struct_array' has incorrect type "
+        "(expected pyarrow.lib.StructArray, got pyarrow.lib.Int64Array)",

Review Comment:
   It seems that we need to add `re.escape()` like `match=re.escape("Argument ...")` because the message includes some special characters for regular expression such as `(` and `.`.
   
   https://github.com/apache/arrow/actions/runs/7170187805/job/19522351324?pr=38520#step:6:5817
   
   ```text
   =================================== FAILURES ===================================
   _____________________ test_table_from_struct_array_invalid _____________________
   
       def test_table_from_struct_array_invalid():
           with pytest.raises(
               TypeError,
               match="Argument 'struct_array' has incorrect type "
               "(expected pyarrow.lib.StructArray, got pyarrow.lib.Int64Array)",
           ):
   >           pa.Table.from_struct_array(pa.array(range(5)))
   
   opt/conda/envs/arrow/lib/python3.9/site-packages/pyarrow/tests/test_table.py:901: 
   _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ 
   
   >   ???
   E   TypeError: Argument 'struct_array' has incorrect type (expected pyarrow.lib.StructArray, got pyarrow.lib.Int64Array)
   
   pyarrow/table.pxi:4022: TypeError
   
   During handling of the above exception, another exception occurred:
   
       def test_table_from_struct_array_invalid():
           with pytest.raises(
               TypeError,
               match="Argument 'struct_array' has incorrect type "
               "(expected pyarrow.lib.StructArray, got pyarrow.lib.Int64Array)",
           ):
   >           pa.Table.from_struct_array(pa.array(range(5)))
   E           AssertionError: Regex pattern did not match.
   E            Regex: "Argument 'struct_array' has incorrect type (expected pyarrow.lib.StructArray, got pyarrow.lib.Int64Array)"
   E            Input: "Argument 'struct_array' has incorrect type (expected pyarrow.lib.StructArray, got pyarrow.lib.Int64Array)"
   E            Did you mean to `re.escape()` the regex?
   
   opt/conda/envs/arrow/lib/python3.9/site-packages/pyarrow/tests/test_table.py:901: AssertionError
   ```



-- 
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


Re: [PR] GH-33500: [Python] add `Table.to/from_struct_array` [arrow]

Posted by "judahrand (via GitHub)" <gi...@apache.org>.
judahrand commented on PR #38520:
URL: https://github.com/apache/arrow/pull/38520#issuecomment-1807989334

   @kou Do you know if there is someone else that I could request a review from? It'd be good to get this merged if possible.


-- 
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


Re: [PR] GH-33500: [Python] add `Table.to/from_struct_array` [arrow]

Posted by "AlenkaF (via GitHub)" <gi...@apache.org>.
AlenkaF commented on code in PR #38520:
URL: https://github.com/apache/arrow/pull/38520#discussion_r1391182668


##########
python/pyarrow/tests/test_table.py:
##########
@@ -878,6 +878,55 @@ def test_recordbatch_from_struct_array():
     ))
 
 
+def test_recordbatch_to_struct_array():
+    batch = pa.RecordBatch.from_arrays(
+        [
+            pa.array([1, None], type=pa.int32()),
+            pa.array([None, 1.0], type=pa.float32()),
+        ], ["ints", "floats"]
+    )
+    result = batch.to_struct_array()
+    assert result.equals(pa.array(
+        [{"ints": 1}, {"floats": 1.0}],
+        type=pa.struct([("ints", pa.int32()), ("floats", pa.float32())]),
+    ))
+
+
+def test_table_from_struct_array_invalid():
+    with pytest.raises(TypeError):

Review Comment:
   Just a nit: could we add a message check here also? It helps reviewing and also later when looking at the tests, like here: https://github.com/apache/arrow/blob/79c72a67f90719fb1af315a9b2b8caa25d667775/python/pyarrow/tests/test_table.py#L800-L802



##########
python/pyarrow/table.pxi:
##########
@@ -2822,7 +2822,7 @@ cdef class RecordBatch(_Tabular):
         4       5  Brittle stars
         5     100      Centipede
 
-        Construct a RecordBartch from pyarrow Arrays using schema:
+        Construct a RecordBatch from pyarrow Arrays using schema:

Review Comment:
   Thanks for catching this! 👍 



##########
python/pyarrow/table.pxi:
##########
@@ -2822,7 +2822,7 @@ cdef class RecordBatch(_Tabular):
         4       5  Brittle stars
         5     100      Centipede
 
-        Construct a RecordBartch from pyarrow Arrays using schema:
+        Construct a RecordBatch from pyarrow Arrays using schema:

Review Comment:
   Thanks for catching this! 👍 



-- 
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


Re: [PR] GH-33500: [Python] add `Table.to/from_struct_array` [arrow]

Posted by "judahrand (via GitHub)" <gi...@apache.org>.
judahrand commented on PR #38520:
URL: https://github.com/apache/arrow/pull/38520#issuecomment-1787093843

   @jorisvandenbossche I think this is ready for a review whenever you have the time. 


-- 
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


Re: [PR] GH-33500: [Python] add `Table.to/from_struct_array` [arrow]

Posted by "judahrand (via GitHub)" <gi...@apache.org>.
judahrand commented on code in PR #38520:
URL: https://github.com/apache/arrow/pull/38520#discussion_r1422716387


##########
python/pyarrow/tests/test_table.py:
##########
@@ -878,6 +878,55 @@ def test_recordbatch_from_struct_array():
     ))
 
 
+def test_recordbatch_to_struct_array():
+    batch = pa.RecordBatch.from_arrays(
+        [
+            pa.array([1, None], type=pa.int32()),
+            pa.array([None, 1.0], type=pa.float32()),
+        ], ["ints", "floats"]
+    )
+    result = batch.to_struct_array()
+    assert result.equals(pa.array(
+        [{"ints": 1}, {"floats": 1.0}],
+        type=pa.struct([("ints", pa.int32()), ("floats", pa.float32())]),
+    ))
+
+
+def test_table_from_struct_array_invalid():
+    with pytest.raises(TypeError):

Review Comment:
   https://github.com/apache/arrow/pull/38520/commits/d260167fb373abcdb9462e7b03c790538f038dc6



##########
python/pyarrow/tests/test_table.py:
##########
@@ -878,6 +878,55 @@ def test_recordbatch_from_struct_array():
     ))
 
 
+def test_recordbatch_to_struct_array():
+    batch = pa.RecordBatch.from_arrays(
+        [
+            pa.array([1, None], type=pa.int32()),
+            pa.array([None, 1.0], type=pa.float32()),
+        ], ["ints", "floats"]
+    )
+    result = batch.to_struct_array()
+    assert result.equals(pa.array(
+        [{"ints": 1}, {"floats": 1.0}],
+        type=pa.struct([("ints", pa.int32()), ("floats", pa.float32())]),
+    ))
+
+
+def test_table_from_struct_array_invalid():
+    with pytest.raises(TypeError):
+        pa.Table.from_struct_array(pa.array(range(5)))
+
+
+def test_table_from_struct_array():
+    struct_array = pa.array(
+        [{"ints": 1}, {"floats": 1.0}],
+        type=pa.struct([("ints", pa.int32()), ("floats", pa.float32())]),
+    )
+    result = pa.Table.from_struct_array(struct_array)
+    assert result.equals(pa.Table.from_arrays(
+        [
+            pa.array([1, None], type=pa.int32()),
+            pa.array([None, 1.0], type=pa.float32()),
+        ], ["ints", "floats"]
+    ))
+
+
+def test_table_to_struct_array():
+    table = pa.Table.from_arrays(
+        [
+            pa.array([1, None], type=pa.int32()),
+            pa.array([None, 1.0], type=pa.float32()),
+        ], ["ints", "floats"]
+    )
+    result = table.to_struct_array()
+    assert result.equals(pa.chunked_array(
+        pa.array(
+            [{"ints": 1}, {"floats": 1.0}],
+            type=pa.struct([("ints", pa.int32()), ("floats", pa.float32())]),
+        ),
+    ))

Review Comment:
   https://github.com/apache/arrow/pull/38520/commits/b4b8d0085fd815191dec818e5cbc52dc7583b62d



-- 
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


Re: [PR] GH-33500: [Python] add `Table.to/from_struct_array` [arrow]

Posted by "judahrand (via GitHub)" <gi...@apache.org>.
judahrand commented on code in PR #38520:
URL: https://github.com/apache/arrow/pull/38520#discussion_r1423745301


##########
python/pyarrow/tests/test_table.py:
##########
@@ -878,6 +878,85 @@ def test_recordbatch_from_struct_array():
     ))
 
 
+def test_recordbatch_to_struct_array():
+    batch = pa.RecordBatch.from_arrays(
+        [
+            pa.array([1, None], type=pa.int32()),
+            pa.array([None, 1.0], type=pa.float32()),
+        ], ["ints", "floats"]
+    )
+    result = batch.to_struct_array()
+    assert result.equals(pa.array(
+        [{"ints": 1}, {"floats": 1.0}],
+        type=pa.struct([("ints", pa.int32()), ("floats", pa.float32())]),
+    ))
+
+
+def test_table_from_struct_array_invalid():
+    with pytest.raises(
+        TypeError,
+        match="Argument 'struct_array' has incorrect type "
+        "(expected pyarrow.lib.StructArray, got pyarrow.lib.Int64Array)",

Review Comment:
   I'm sorry, that's embarrassing! I didn't loop around to get my `pyarrow` dev environment working again so was making blind changes but I should have caught this! 



-- 
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


Re: [PR] GH-33500 [python] add `Table.to/from_struct_array` [arrow]

Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on PR #38520:
URL: https://github.com/apache/arrow/pull/38520#issuecomment-1786172827

   :warning: GitHub issue #33500 **has been automatically assigned in GitHub** to PR creator.


-- 
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


Re: [PR] GH-33500: [Python] add `Table.to/from_struct_array` [arrow]

Posted by "judahrand (via GitHub)" <gi...@apache.org>.
judahrand commented on PR #38520:
URL: https://github.com/apache/arrow/pull/38520#issuecomment-1850376013

   @AlenkaF @kou Sorry it took me so long to get to your feedback! It should all be dealt with now.


-- 
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


Re: [PR] GH-33500: [Python] add `Table.to/from_struct_array` [arrow]

Posted by "kou (via GitHub)" <gi...@apache.org>.
kou commented on code in PR #38520:
URL: https://github.com/apache/arrow/pull/38520#discussion_r1423120960


##########
python/pyarrow/tests/test_table.py:
##########
@@ -878,6 +878,85 @@ def test_recordbatch_from_struct_array():
     ))
 
 
+def test_recordbatch_to_struct_array():
+    batch = pa.RecordBatch.from_arrays(
+        [
+            pa.array([1, None], type=pa.int32()),
+            pa.array([None, 1.0], type=pa.float32()),
+        ], ["ints", "floats"]
+    )
+    result = batch.to_struct_array()
+    assert result.equals(pa.array(
+        [{"ints": 1}, {"floats": 1.0}],
+        type=pa.struct([("ints", pa.int32()), ("floats", pa.float32())]),
+    ))
+
+
+def test_table_from_struct_array_invalid():
+    with pytest.raises(
+        TypeError,
+        match="Argument 'struct_array' has incorrect type "
+        "(expected pyarrow.lib.StructArray, got pyarrow.lib.Int64Array)",
+    ):
+        pa.Table.from_struct_array(pa.array(range(5)))
+
+
+def test_table_from_struct_array():
+    struct_array = pa.array(
+        [{"ints": 1}, {"floats": 1.0}],
+        type=pa.struct([("ints", pa.int32()), ("floats", pa.float32())]),
+    )
+    result = pa.Table.from_struct_array(struct_array)
+    assert result.equals(pa.Table.from_arrays(
+        [
+            pa.array([1, None], type=pa.int32()),
+            pa.array([None, 1.0], type=pa.float32()),
+        ], ["ints", "floats"]
+    ))
+
+
+def test_table_from_struct_array_chunked_array():
+    chunked_struct_array = pa.chunked_array(
+        [{"ints": 1}, {"floats": 1.0}],
+        type=pa.struct([("ints", pa.int32()), ("floats", pa.float32())]),
+    )

Review Comment:
   Could you check the failure?
   
   https://github.com/apache/arrow/actions/runs/7170187805/job/19522351324?pr=38520#step:6:5851
   
   ```text
   __________________ test_table_from_struct_array_chunked_array __________________
   
       def test_table_from_struct_array_chunked_array():
   >       chunked_struct_array = pa.chunked_array(
               [{"ints": 1}, {"floats": 1.0}],
               type=pa.struct([("ints", pa.int32()), ("floats", pa.float32())]),
           )
   
   opt/conda/envs/arrow/lib/python3.9/site-packages/pyarrow/tests/test_table.py:919: 
   _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ 
   pyarrow/table.pxi:1381: in pyarrow.lib.chunked_array
       ???
   pyarrow/array.pxi:344: in pyarrow.lib.array
       ???
   pyarrow/array.pxi:42: in pyarrow.lib._sequence_to_array
       ???
   pyarrow/error.pxi:154: in pyarrow.lib.pyarrow_internal_check_status
       ???
   _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ 
   
   >   ???
   E   pyarrow.lib.ArrowTypeError: Could not convert 'i' with type str: was expecting tuple of (key, value) pair
   
   pyarrow/error.pxi:91: ArrowTypeError
   ```



-- 
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


Re: [PR] GH-33500: [Python] add `Table.to/from_struct_array` [arrow]

Posted by "jorisvandenbossche (via GitHub)" <gi...@apache.org>.
jorisvandenbossche merged PR #38520:
URL: https://github.com/apache/arrow/pull/38520


-- 
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


Re: [PR] GH-33500: [Python] add `Table.to/from_struct_array` [arrow]

Posted by "judahrand (via GitHub)" <gi...@apache.org>.
judahrand commented on code in PR #38520:
URL: https://github.com/apache/arrow/pull/38520#discussion_r1422716152


##########
python/pyarrow/table.pxi:
##########
@@ -3986,6 +3986,59 @@ cdef class Table(_Tabular):
         result.validate()
         return result
 
+    @staticmethod
+    def from_struct_array(struct_array):
+        """
+        Construct a Table from a StructArray.
+
+        Each field in the StructArray will become a column in the resulting
+        ``Table``.
+
+        Parameters
+        ----------
+        struct_array : StructArray or ChunkedArray
+            Array to construct the table from.
+
+        Returns
+        -------
+        pyarrow.Table
+
+        Examples
+        --------
+        >>> import pyarrow as pa
+        >>> struct = pa.array([{'n_legs': 2, 'animals': 'Parrot'},
+        ...                    {'year': 2022, 'n_legs': 4}])
+        >>> pa.Table.from_struct_array(struct).to_pandas()
+          animals  n_legs    year
+        0  Parrot       2     NaN
+        1    None       4  2022.0
+        """
+        if isinstance(struct_array, Array):
+            struct_array = chunked_array([struct_array])
+        return Table.from_batches([
+            RecordBatch.from_struct_array(chunk)
+            for chunk in struct_array.chunks
+        ])

Review Comment:
   https://github.com/apache/arrow/pull/38520/commits/52a792a6d6b371322c3bc860301ecb21467afbe0



-- 
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


Re: [PR] GH-33500: [Python] add `Table.to/from_struct_array` [arrow]

Posted by "judahrand (via GitHub)" <gi...@apache.org>.
judahrand commented on code in PR #38520:
URL: https://github.com/apache/arrow/pull/38520#discussion_r1422716598


##########
python/pyarrow/tests/test_table.py:
##########
@@ -878,6 +878,55 @@ def test_recordbatch_from_struct_array():
     ))
 
 
+def test_recordbatch_to_struct_array():
+    batch = pa.RecordBatch.from_arrays(
+        [
+            pa.array([1, None], type=pa.int32()),
+            pa.array([None, 1.0], type=pa.float32()),
+        ], ["ints", "floats"]
+    )
+    result = batch.to_struct_array()
+    assert result.equals(pa.array(
+        [{"ints": 1}, {"floats": 1.0}],
+        type=pa.struct([("ints", pa.int32()), ("floats", pa.float32())]),
+    ))
+
+
+def test_table_from_struct_array_invalid():
+    with pytest.raises(TypeError):
+        pa.Table.from_struct_array(pa.array(range(5)))
+
+
+def test_table_from_struct_array():
+    struct_array = pa.array(
+        [{"ints": 1}, {"floats": 1.0}],
+        type=pa.struct([("ints", pa.int32()), ("floats", pa.float32())]),
+    )
+    result = pa.Table.from_struct_array(struct_array)
+    assert result.equals(pa.Table.from_arrays(
+        [
+            pa.array([1, None], type=pa.int32()),
+            pa.array([None, 1.0], type=pa.float32()),
+        ], ["ints", "floats"]
+    ))
+
+
+def test_table_to_struct_array():
+    table = pa.Table.from_arrays(
+        [
+            pa.array([1, None], type=pa.int32()),
+            pa.array([None, 1.0], type=pa.float32()),
+        ], ["ints", "floats"]
+    )
+    result = table.to_struct_array()
+    assert result.equals(pa.chunked_array(
+        pa.array(
+            [{"ints": 1}, {"floats": 1.0}],
+            type=pa.struct([("ints", pa.int32()), ("floats", pa.float32())]),
+        ),
+    ))

Review Comment:
   https://github.com/apache/arrow/pull/38520/commits/2c0ab6716175b50ce39cdf78a02ee2e78c36b52c



-- 
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


Re: [PR] GH-33500: [Python] add `Table.to/from_struct_array` [arrow]

Posted by "judahrand (via GitHub)" <gi...@apache.org>.
judahrand commented on code in PR #38520:
URL: https://github.com/apache/arrow/pull/38520#discussion_r1422716152


##########
python/pyarrow/table.pxi:
##########
@@ -3986,6 +3986,59 @@ cdef class Table(_Tabular):
         result.validate()
         return result
 
+    @staticmethod
+    def from_struct_array(struct_array):
+        """
+        Construct a Table from a StructArray.
+
+        Each field in the StructArray will become a column in the resulting
+        ``Table``.
+
+        Parameters
+        ----------
+        struct_array : StructArray or ChunkedArray
+            Array to construct the table from.
+
+        Returns
+        -------
+        pyarrow.Table
+
+        Examples
+        --------
+        >>> import pyarrow as pa
+        >>> struct = pa.array([{'n_legs': 2, 'animals': 'Parrot'},
+        ...                    {'year': 2022, 'n_legs': 4}])
+        >>> pa.Table.from_struct_array(struct).to_pandas()
+          animals  n_legs    year
+        0  Parrot       2     NaN
+        1    None       4  2022.0
+        """
+        if isinstance(struct_array, Array):
+            struct_array = chunked_array([struct_array])
+        return Table.from_batches([
+            RecordBatch.from_struct_array(chunk)
+            for chunk in struct_array.chunks
+        ])

Review Comment:
   https://github.com/apache/arrow/pull/38520/commits/93c54d3f8d390b7b4e5b28b3451275108d4ea567



-- 
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


Re: [PR] GH-33500: [Python] add `Table.to/from_struct_array` [arrow]

Posted by "judahrand (via GitHub)" <gi...@apache.org>.
judahrand commented on code in PR #38520:
URL: https://github.com/apache/arrow/pull/38520#discussion_r1423772108


##########
python/pyarrow/table.pxi:
##########
@@ -3986,6 +3986,60 @@ cdef class Table(_Tabular):
         result.validate()
         return result
 
+    @staticmethod
+    def from_struct_array(struct_array):
+        """
+        Construct a Table from a StructArray.
+
+        Each field in the StructArray will become a column in the resulting
+        ``Table``.
+
+        Parameters
+        ----------
+        struct_array : StructArray or ChunkedArray
+            Array to construct the table from.
+
+        Returns
+        -------
+        pyarrow.Table
+
+        Examples
+        --------
+        >>> import pyarrow as pa
+        >>> struct = pa.array([{'n_legs': 2, 'animals': 'Parrot'},
+        ...                    {'year': 2022, 'n_legs': 4}])
+        >>> pa.Table.from_struct_array(struct).to_pandas()
+          animals  n_legs    year
+        0  Parrot       2     NaN
+        1    None       4  2022.0
+        """
+        if isinstance(struct_array, Array):
+            return Table.from_batches([RecordBatch.from_struct_array(struct_array)])
+        else:
+            return Table.from_batches([
+                RecordBatch.from_struct_array(chunk)
+                for chunk in struct_array.chunks
+            ])
+
+    def to_struct_array(self, max_chunksize=None):
+        """
+        Convert to a struct array.

Review Comment:
   https://github.com/apache/arrow/pull/38520/commits/4ff720a9f97bda785b0697d5144f61b529e94b4e



-- 
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


Re: [PR] GH-33500: [Python] add `Table.to/from_struct_array` [arrow]

Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on PR #38520:
URL: https://github.com/apache/arrow/pull/38520#issuecomment-1786551012

   :warning: GitHub issue #33500 **has been automatically assigned in GitHub** to PR creator.


-- 
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


Re: [PR] GH-33500: [Python] add `Table.to/from_struct_array` [arrow]

Posted by "kou (via GitHub)" <gi...@apache.org>.
kou commented on code in PR #38520:
URL: https://github.com/apache/arrow/pull/38520#discussion_r1391133051


##########
python/pyarrow/table.pxi:
##########
@@ -3986,6 +3986,59 @@ cdef class Table(_Tabular):
         result.validate()
         return result
 
+    @staticmethod
+    def from_struct_array(struct_array):
+        """
+        Construct a Table from a StructArray.
+
+        Each field in the StructArray will become a column in the resulting
+        ``Table``.
+
+        Parameters
+        ----------
+        struct_array : StructArray or ChunkedArray
+            Array to construct the table from.
+
+        Returns
+        -------
+        pyarrow.Table
+
+        Examples
+        --------
+        >>> import pyarrow as pa
+        >>> struct = pa.array([{'n_legs': 2, 'animals': 'Parrot'},
+        ...                    {'year': 2022, 'n_legs': 4}])
+        >>> pa.Table.from_struct_array(struct).to_pandas()
+          animals  n_legs    year
+        0  Parrot       2     NaN
+        1    None       4  2022.0
+        """
+        if isinstance(struct_array, Array):
+            struct_array = chunked_array([struct_array])
+        return Table.from_batches([
+            RecordBatch.from_struct_array(chunk)
+            for chunk in struct_array.chunks
+        ])

Review Comment:
   Can we avoid creating a needless chunked array?
   
   ```suggestion
           if isinstance(struct_array, Array):
               return Table.from_batches([RecordBatch.from_struct_array(struct_array)])
           else:
               return Table.from_batches([
                   RecordBatch.from_struct_array(chunk)
                   for chunk in struct_array.chunks
               ])
   ```



##########
python/pyarrow/tests/test_table.py:
##########
@@ -878,6 +878,55 @@ def test_recordbatch_from_struct_array():
     ))
 
 
+def test_recordbatch_to_struct_array():
+    batch = pa.RecordBatch.from_arrays(
+        [
+            pa.array([1, None], type=pa.int32()),
+            pa.array([None, 1.0], type=pa.float32()),
+        ], ["ints", "floats"]
+    )
+    result = batch.to_struct_array()
+    assert result.equals(pa.array(
+        [{"ints": 1}, {"floats": 1.0}],
+        type=pa.struct([("ints", pa.int32()), ("floats", pa.float32())]),
+    ))
+
+
+def test_table_from_struct_array_invalid():
+    with pytest.raises(TypeError):
+        pa.Table.from_struct_array(pa.array(range(5)))
+
+
+def test_table_from_struct_array():
+    struct_array = pa.array(
+        [{"ints": 1}, {"floats": 1.0}],
+        type=pa.struct([("ints", pa.int32()), ("floats", pa.float32())]),
+    )
+    result = pa.Table.from_struct_array(struct_array)
+    assert result.equals(pa.Table.from_arrays(
+        [
+            pa.array([1, None], type=pa.int32()),
+            pa.array([None, 1.0], type=pa.float32()),
+        ], ["ints", "floats"]
+    ))
+
+
+def test_table_to_struct_array():
+    table = pa.Table.from_arrays(
+        [
+            pa.array([1, None], type=pa.int32()),
+            pa.array([None, 1.0], type=pa.float32()),
+        ], ["ints", "floats"]
+    )
+    result = table.to_struct_array()
+    assert result.equals(pa.chunked_array(
+        pa.array(
+            [{"ints": 1}, {"floats": 1.0}],
+            type=pa.struct([("ints", pa.int32()), ("floats", pa.float32())]),
+        ),
+    ))

Review Comment:
   Can we also add a test with `max_chunksize`?



##########
python/pyarrow/tests/test_table.py:
##########
@@ -878,6 +878,55 @@ def test_recordbatch_from_struct_array():
     ))
 
 
+def test_recordbatch_to_struct_array():
+    batch = pa.RecordBatch.from_arrays(
+        [
+            pa.array([1, None], type=pa.int32()),
+            pa.array([None, 1.0], type=pa.float32()),
+        ], ["ints", "floats"]
+    )
+    result = batch.to_struct_array()
+    assert result.equals(pa.array(
+        [{"ints": 1}, {"floats": 1.0}],
+        type=pa.struct([("ints", pa.int32()), ("floats", pa.float32())]),
+    ))
+
+
+def test_table_from_struct_array_invalid():
+    with pytest.raises(TypeError):
+        pa.Table.from_struct_array(pa.array(range(5)))
+
+
+def test_table_from_struct_array():
+    struct_array = pa.array(
+        [{"ints": 1}, {"floats": 1.0}],
+        type=pa.struct([("ints", pa.int32()), ("floats", pa.float32())]),
+    )
+    result = pa.Table.from_struct_array(struct_array)
+    assert result.equals(pa.Table.from_arrays(
+        [
+            pa.array([1, None], type=pa.int32()),
+            pa.array([None, 1.0], type=pa.float32()),
+        ], ["ints", "floats"]
+    ))

Review Comment:
   Can we add `test_table_from_struct_array_chunked_array()` like this?



-- 
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


Re: [PR] GH-33500: [Python] add `Table.to/from_struct_array` [arrow]

Posted by "judahrand (via GitHub)" <gi...@apache.org>.
judahrand commented on code in PR #38520:
URL: https://github.com/apache/arrow/pull/38520#discussion_r1422716863


##########
python/pyarrow/tests/test_table.py:
##########
@@ -878,6 +878,55 @@ def test_recordbatch_from_struct_array():
     ))
 
 
+def test_recordbatch_to_struct_array():
+    batch = pa.RecordBatch.from_arrays(
+        [
+            pa.array([1, None], type=pa.int32()),
+            pa.array([None, 1.0], type=pa.float32()),
+        ], ["ints", "floats"]
+    )
+    result = batch.to_struct_array()
+    assert result.equals(pa.array(
+        [{"ints": 1}, {"floats": 1.0}],
+        type=pa.struct([("ints", pa.int32()), ("floats", pa.float32())]),
+    ))
+
+
+def test_table_from_struct_array_invalid():
+    with pytest.raises(TypeError):
+        pa.Table.from_struct_array(pa.array(range(5)))
+
+
+def test_table_from_struct_array():
+    struct_array = pa.array(
+        [{"ints": 1}, {"floats": 1.0}],
+        type=pa.struct([("ints", pa.int32()), ("floats", pa.float32())]),
+    )
+    result = pa.Table.from_struct_array(struct_array)
+    assert result.equals(pa.Table.from_arrays(
+        [
+            pa.array([1, None], type=pa.int32()),
+            pa.array([None, 1.0], type=pa.float32()),
+        ], ["ints", "floats"]
+    ))

Review Comment:
   https://github.com/apache/arrow/pull/38520/commits/33b3d088e591f491e69f4d4d43ec98bff89f54a2



-- 
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


Re: [PR] GH-33500: [Python] add `Table.to/from_struct_array` [arrow]

Posted by "judahrand (via GitHub)" <gi...@apache.org>.
judahrand commented on code in PR #38520:
URL: https://github.com/apache/arrow/pull/38520#discussion_r1422716387


##########
python/pyarrow/tests/test_table.py:
##########
@@ -878,6 +878,55 @@ def test_recordbatch_from_struct_array():
     ))
 
 
+def test_recordbatch_to_struct_array():
+    batch = pa.RecordBatch.from_arrays(
+        [
+            pa.array([1, None], type=pa.int32()),
+            pa.array([None, 1.0], type=pa.float32()),
+        ], ["ints", "floats"]
+    )
+    result = batch.to_struct_array()
+    assert result.equals(pa.array(
+        [{"ints": 1}, {"floats": 1.0}],
+        type=pa.struct([("ints", pa.int32()), ("floats", pa.float32())]),
+    ))
+
+
+def test_table_from_struct_array_invalid():
+    with pytest.raises(TypeError):

Review Comment:
   https://github.com/apache/arrow/pull/38520/commits/c2bbf7d25100843860923f28ae4f8ab09fe0d565



##########
python/pyarrow/tests/test_table.py:
##########
@@ -878,6 +878,55 @@ def test_recordbatch_from_struct_array():
     ))
 
 
+def test_recordbatch_to_struct_array():
+    batch = pa.RecordBatch.from_arrays(
+        [
+            pa.array([1, None], type=pa.int32()),
+            pa.array([None, 1.0], type=pa.float32()),
+        ], ["ints", "floats"]
+    )
+    result = batch.to_struct_array()
+    assert result.equals(pa.array(
+        [{"ints": 1}, {"floats": 1.0}],
+        type=pa.struct([("ints", pa.int32()), ("floats", pa.float32())]),
+    ))
+
+
+def test_table_from_struct_array_invalid():
+    with pytest.raises(TypeError):
+        pa.Table.from_struct_array(pa.array(range(5)))
+
+
+def test_table_from_struct_array():
+    struct_array = pa.array(
+        [{"ints": 1}, {"floats": 1.0}],
+        type=pa.struct([("ints", pa.int32()), ("floats", pa.float32())]),
+    )
+    result = pa.Table.from_struct_array(struct_array)
+    assert result.equals(pa.Table.from_arrays(
+        [
+            pa.array([1, None], type=pa.int32()),
+            pa.array([None, 1.0], type=pa.float32()),
+        ], ["ints", "floats"]
+    ))

Review Comment:
   https://github.com/apache/arrow/pull/38520/commits/5075fdef710324db3143289bd95baaead24d047e



-- 
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


Re: [PR] GH-33500: [Python] add `Table.to/from_struct_array` [arrow]

Posted by "jorisvandenbossche (via GitHub)" <gi...@apache.org>.
jorisvandenbossche commented on code in PR #38520:
URL: https://github.com/apache/arrow/pull/38520#discussion_r1423760032


##########
python/pyarrow/table.pxi:
##########
@@ -3986,6 +3986,60 @@ cdef class Table(_Tabular):
         result.validate()
         return result
 
+    @staticmethod
+    def from_struct_array(struct_array):
+        """
+        Construct a Table from a StructArray.
+
+        Each field in the StructArray will become a column in the resulting
+        ``Table``.
+
+        Parameters
+        ----------
+        struct_array : StructArray or ChunkedArray
+            Array to construct the table from.
+
+        Returns
+        -------
+        pyarrow.Table
+
+        Examples
+        --------
+        >>> import pyarrow as pa
+        >>> struct = pa.array([{'n_legs': 2, 'animals': 'Parrot'},
+        ...                    {'year': 2022, 'n_legs': 4}])
+        >>> pa.Table.from_struct_array(struct).to_pandas()
+          animals  n_legs    year
+        0  Parrot       2     NaN
+        1    None       4  2022.0
+        """
+        if isinstance(struct_array, Array):
+            return Table.from_batches([RecordBatch.from_struct_array(struct_array)])
+        else:
+            return Table.from_batches([
+                RecordBatch.from_struct_array(chunk)
+                for chunk in struct_array.chunks
+            ])
+
+    def to_struct_array(self, max_chunksize=None):
+        """
+        Convert to a struct array.

Review Comment:
   Maybe we can be more explicit that it will be a chunked array of struct type?



##########
python/pyarrow/tests/test_table.py:
##########
@@ -878,6 +879,87 @@ def test_recordbatch_from_struct_array():
     ))
 
 
+def test_recordbatch_to_struct_array():
+    batch = pa.RecordBatch.from_arrays(
+        [
+            pa.array([1, None], type=pa.int32()),
+            pa.array([None, 1.0], type=pa.float32()),
+        ], ["ints", "floats"]
+    )
+    result = batch.to_struct_array()
+    assert result.equals(pa.array(
+        [{"ints": 1}, {"floats": 1.0}],
+        type=pa.struct([("ints", pa.int32()), ("floats", pa.float32())]),
+    ))
+
+
+def test_table_from_struct_array_invalid():
+    with pytest.raises(
+        TypeError,
+        match=re.escape(
+            "Argument 'struct_array' has incorrect type "
+            "(expected pyarrow.lib.StructArray, got pyarrow.lib.Int64Array)",
+        ),
+    ):

Review Comment:
   ```suggestion
       with pytest.raises(TypeError, match="Argument 'struct_array' has incorrect type"):
   ```
   
   This is a bit simpler, and more than good enough I think (the actual content of the error message is generated by cython)



-- 
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


Re: [PR] GH-33500: [Python] add `Table.to/from_struct_array` [arrow]

Posted by "conbench-apache-arrow[bot] (via GitHub)" <gi...@apache.org>.
conbench-apache-arrow[bot] commented on PR #38520:
URL: https://github.com/apache/arrow/pull/38520#issuecomment-1882617347

   After merging your PR, Conbench analyzed the 6 benchmarking runs that have been run so far on merge-commit 60b89ff0c94d695b05997f6802efdc148bcbd3d7.
   
   There were no benchmark performance regressions. 🎉
   
   The [full Conbench report](https://github.com/apache/arrow/runs/20292780500) has more details. It also includes information about 3 possible false positives for unstable benchmarks that are known to sometimes produce them.


-- 
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