ENH: raise Python warnings instead of printing to stderr for GDAL Warning messages #242
Conversation
There was a problem hiding this comment.
Thanks @jorisvandenbossche !
Can you please add a changelog entry, and also edit the docstring for kwargs of read_info,read, and read_dataframe to remove mention of logging invalid options to stderr?
|
I updated the tests a bit to avoid warnings being raised from running them. But how to exactly do this can certainly be discussed. For example, in some cases I could keep it simpler and just add some more |
| @@ -270,6 +271,7 @@ def test_write(tmpdir, naturalearth_lowres): | |||
|
|
|||
| def test_write_gpkg(tmpdir, naturalearth_lowres): | |||
| meta, _, geometry, field_data = read(naturalearth_lowres) | |||
| meta.update({"geometry_type": "MultiPolygon"}) | |||
There was a problem hiding this comment.
The problem here is that reading the Shapefile will indicate the geometry type is "Polygon" (because shapefiles don't care about the distinction), but then using that meta to write to geopackage will raise a warning because geopackage is stricter
| @@ -456,9 +458,11 @@ def assert_equal_result(result1, result2): | |||
| assert all([np.array_equal(f1, f2) for f1, f2 in zip(field_data1, field_data2)]) | |||
|
|
|||
|
|
|||
| @pytest.mark.filterwarnings("ignore:File /vsimem:RuntimeWarning") # TODO | |||
There was a problem hiding this comment.
When reading from bytes / file-like objects, we create a vsimem temporary in-memory file, but because we don't know the file type at that moment, we get a warning like "File /vsimem/5332805636f74498b74520a2c132082b has GPKG application_id, but non conformant file extension"
Ideally we would solve that (since the user cannot do anything about that warning, since this name is auto-generated), but not sure how. We don't know the type of file when it's a file-like object, and it's only GDAL that infers it based on the content of the file. Maybe we should open an issue about upstream in GDAL to relax this constraint for vsimem files.
| if dtype == "float32": | ||
| ctx = pytest.warns(RuntimeWarning, match="NaN of Infinity value found. Skipped") | ||
| else: | ||
| ctx = contextlib.nullcontext() |
There was a problem hiding this comment.
This is bizarre, but for some reason GDAL only raise the warning if the dtype is float32, while the value is actually skipped for both float32 and float64
There was a problem hiding this comment.
Seems like an issue to raise upstream?
There was a problem hiding this comment.
Ah, so the issue seems to be that it only warns once, not depending on float32 vs float64. I would expect it to warn once per file, and not once per session, though
There was a problem hiding this comment.
Thanks @jorisvandenbossche !
I think your approach for the tests is good (testing for specific warnings instead of filtering). The issues that come from GDAL don't need to be resolved to merge this.
Co-authored-by: Brendan Ward <bcward@astutespruce.com>
Follow-up on #236, xref #91 (comment)