Default to handle float NaN as null + add option to control it #190
Conversation
There was a problem hiding this comment.
Thanks for working on this @jorisvandenbossche !
A few minor comments to consider but otherwise this is ready to merge.
| @@ -522,6 +522,72 @@ def test_read_write_null_geometry(tmp_path, ext): | |||
| assert np.array_equal(result_fields[0], field_data[0]) | |||
|
|
|||
|
|
|||
| def test_write_float_nan_null(tmp_path): | |||
| geometry = np.array( | |||
There was a problem hiding this comment.
Please add comments denoting the WKT when using geometries created from hex (I know we use the same one elsewhere but it is helpful each time).
pyogrio/tests/test_raw_io.py
Outdated
| content = f.read() | ||
| assert '{ "col": NaN }' in content | ||
|
|
||
| # by default, GDAL will skip the property for GeoJSON if the value is NaN |
There was a problem hiding this comment.
Suggest moving this before the case starting on 541 (so order is default GDAL behavior then override of that) and adding a comment that WRITE_NON_FINITE_VALUES="YES" is required to force GDAL to write that as NaN.
pyogrio/tests/test_raw_io.py
Outdated
| table = pyarrow.feather.read_table(fname) | ||
| assert table["col"].is_null().to_pylist() == [False, True] | ||
|
|
||
| # default nan_as_null=True |
There was a problem hiding this comment.
| # default nan_as_null=True |
(comment is not correct)
pyogrio/tests/test_raw_io.py
Outdated
| [bytes.fromhex("010100000000000000000000000000000000000000")] * 2, | ||
| dtype=object, | ||
| ) | ||
| field_data = [np.array([1.5, np.nan], dtype="float64")] |
There was a problem hiding this comment.
Just to make sure we have it covered, would you mind parametrizing this test with both float64 and float32?
Closes #122