[GitHub] betodealmeida commented on a change in pull request #4908: Replace NaN/Infinity with null

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

betodealmeida commented on a change in pull request #4908: Replace NaN/Infinity with null
URL: https://github.com/apache/incubator-superset/pull/4908#discussion_r185049884
 
 

 ##########
 File path: superset/views/core.py
 ##########
 @@ -2435,7 +2436,7 @@ def sql_json(self):
 
             resp = json_success(json.dumps(
                 {'query': query.to_dict()}, default=utils.json_int_dttm_ser,
-                allow_nan=False), status=202)
+                ignore_nan=True), status=202)
 
 Review comment:
   Yeah. This is super confusing naming. Just to clarify:
   
   - `allow_nan=False`: will raise an exception if the data has NaN/Infinity;
   - `ignore_nan=True`: will encode NaN/Infinity as nulls.
   
   I'm assuming what happened was that someone had an async query returning NaN/Infinity, and since the stdlib `json` module does not have the `ignore_nan` argument the easiest way is to work around is to raise an exception here. It took me a while to learn that `simplejson` had this option.
   
   I looked at the blame and couldn't find any info about this. The reason I changed this as well was to make it consistent with the sync call.

----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services