[GitHub] [arrow] jorisvandenbossche commented on a diff in pull request #12791: ARROW-16113: [Python] Partitioning.dictionaries in case of a subset of fields are dictionary encoded

[GitBox <gi...@apache.org>]

jorisvandenbossche commented on code in PR #12791:
URL: https://github.com/apache/arrow/pull/12791#discussion_r842409863


##########
python/pyarrow/tests/test_dataset.py:
##########
@@ -549,7 +549,8 @@ def test_partitioning():
             pa.field('key', pa.float64())
         ])
     )
-    assert len(partitioning.dictionaries) == 0
+    assert len(partitioning.dictionaries) == 2
+    assert all(x is None for x in partitioning.dictionaries) is True

Review Comment:
   ```suggestion
       assert all(x is None for x in partitioning.dictionaries)
   ```
   
   One minor comment: when doing an `assert all(...)`, the `is True` is not needed (`all` will return True or False, and `assert` will check that it is True)



##########
python/pyarrow/tests/test_dataset.py:
##########
@@ -623,7 +626,7 @@ def test_partitioning():
         dictionaries={
             "key": pa.array(["first", "second", "third"]),
         })
-    assert partitioning.dictionaries[0].to_pylist() == [
+    assert partitioning.dictionaries[1].to_pylist() == [

Review Comment:
   Can you also assert here that the first element is `None` (`assert partitioning.dictionaries[0] is None`)



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