[GitHub] [arrow] mikelui commented on a diff in pull request #34730: GH-34729: [C++, Python] Enhanced Arrow<->Pandas map/pydict support

["mikelui (via GitHub)" <gi...@apache.org>]

mikelui commented on code in PR #34730:
URL: https://github.com/apache/arrow/pull/34730#discussion_r1148480831


##########
python/pyarrow/src/arrow/python/python_to_arrow.cc:
##########
@@ -762,7 +762,7 @@ class PyListConverter : public ListConverter<T, PyConverter, PyConverterTrait> {
       RETURN_NOT_OK(AppendSequence(value));
     } else if (PySet_Check(value) || (Py_TYPE(value) == &PyDictValues_Type)) {
       RETURN_NOT_OK(AppendIterable(value));
-    } else if (PyDict_Check(value) && this->options_.type->id() == Type::MAP) {
+    } else if (PyDict_Check(value) && this->type()->id() == Type::MAP) {

Review Comment:
   It looks like `options_` isn't updated/set as expected in `MakeConverter`
   
   So, when there's a nested map (e.g. list of map):
   1. `type_` is updated to be the list's value_type, but
   2. `option_` is passed along unchanged and still has its `type_` set as the list
   
   That causes this check to fail unexpected, since the pydict is seen as having `options_.type->id() == Type::LIST` instead of `type()->id() == Type::MAP`
   
   I am unsure what the expected behavior of `options_` is, but this check certainly is meant to check the current type.



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