[GitHub] [superset] eric-briscoe commented on a diff in pull request #22881: fix: edit pyarrow stringify to better handle emojis and accents

["eric-briscoe (via GitHub)" <gi...@apache.org>]

eric-briscoe commented on code in PR #22881:
URL: https://github.com/apache/superset/pull/22881#discussion_r1089582743


##########
superset/result_set.py:
##########
@@ -70,9 +70,14 @@ def stringify_values(array: NDArray[Any]) -> NDArray[Any]:
         for obj in it:
             if na_obj := pd.isna(obj):
                 # pandas <NA> type cannot be converted to string
-                obj[na_obj] = None  # type: ignore
+                obj[na_obj] = None
             else:
-                obj[...] = stringify(obj)  # type: ignore
+                try:
+                    # for simple string conversions
+                    # this handles odd character types better
+                    obj[...] = obj.astype(str)

Review Comment:
   @eschutho There are some failed unit tests comparing stringified objects.  It seems like the 
   `obj.astype(str)` call may return inverted use of single and double quotes from `stringify(obj)`
   example from failed tests:
   returned: 
   `"{'chart_name': 'scatter'}"` <-- not technically valid JSON
   and 
   Expected: 
   `'{"chart_name": "scatter"}'` <-- valid JSON
   
   Not sure if this matters in actual python code execution, but from a JSON point of view the new stringified output is not valid to use in JSON.parse() and would throw JS error
   
   Regardless of if it matters if there is valid JSON here, the unit tests that were in place. before this change are in the valid JSON format and are failing because of the quote inversion. 
   
   https://github.com/apache/superset/blob/d954c3df8604dc4b6a2459a17dd39450a1d57638/tests/integration_tests/result_set_tests.py#L165



-- 
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: notifications-unsubscribe@superset.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org