Replace NaN/Infinity with null

[betodealmeida]

The Python json module will happily encode NaN and ±Infinity:

>>> import json
>>> json.dumps([float('nan'), float('inf'), float('-inf')])
'[NaN, Infinity, -Infinity]'

The problem is that this is not valid JSON, and the browser will choke on the payload when running JSON.parse on it:

> JSON.parse('[NaN, Infinity, -Infinity]');
Uncaught SyntaxError: Unexpected token N in JSON at position 1
    at JSON.parse (<anonymous>)
    at <anonymous>:1:6

To fix this, I used the simplejson module instead, since it provides an argument ignore_nan for converting these values to null as recommended by the JSON spec:

>>> import simplejson as json
>>> json.dumps([float('nan'), float('inf'), float('-inf')], ignore_nan=True)
'[null, null, null]'

[mistercrunch]

This is a little bit scary as it was set to False explicitly in the past, probably for a reason. I'm guessing the default is True and someone set it to False for some specific reason. We should do a bit of git forensics to understand how it came to be.

[betodealmeida]

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.

[betodealmeida]

Digging in a bit more, I found the first commit where this was introduced by some guy called @mistercrunch: 38b8db805 :-P

Initially both sync and async responses used allow_nan=False. Eventually it got dropped from the sync response in 269f55c29 when the pessimistic encoder was introduced. The pessimistic encoder was not added to the async response because the commit is trying to fix gigantic HTML error messages, but IMHO it should've been added as well.

The problem is that the pessimistic encoder doesn't handle NaN and ±Infinity because they're floats, so the JSON encoder never calls the default method on them.

I think the change above is safe — it will return some result instead of failing with a ValueError exception. But I think the best approach would be using a more robust serialization to send data to the browser, like BSON.

[codecov-io]

Codecov Report

Merging #4908 into master will decrease coverage by 0.01%.
The diff coverage is 100%.

@@            Coverage Diff             @@
##           master    #4908      +/-   ##
==========================================
- Coverage   77.13%   77.12%   -0.02%     
==========================================
  Files          44       44              
  Lines        8542     8542              
==========================================
- Hits         6589     6588       -1     
- Misses       1953     1954       +1

Impacted Files	Coverage Δ
superset/views/core.py	74.68% <100%> (ø)	⬆️
superset/utils.py	87.85% <0%> (-0.22%)	⬇️

---

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update d533ce0...1a66e63. Read the comment docs.