Skip to content

fix(csvjson): a blank --lat/--lon crashes instead of writing a null geometry - #1355

Open
VXNCXNX wants to merge 2 commits into
wireservice:masterfrom
VXNCXNX:fix/geojson-null-geometry
Open

fix(csvjson): a blank --lat/--lon crashes instead of writing a null geometry#1355
VXNCXNX wants to merge 2 commits into
wireservice:masterfrom
VXNCXNX:fix/geojson-null-geometry

Conversation

@VXNCXNX

@VXNCXNX VXNCXNX commented Aug 15, 2026

Copy link
Copy Markdown

What's broken

csvjson --lat/--lon aborts on any row whose coordinates are blank.

$ printf 'name,lat,lon\nparis,48.85,2.35\nunknown,,\n' > geo.csv
$ csvjson --lat lat --lon lon geo.csv
TypeError: float() argument must be a string or a real number, not 'NoneType'

A blank coordinate is the ordinary case in a geocoded export: one address failed to resolve and the rest are fine. One such row loses the whole file.

After:

{"type": "FeatureCollection", "bbox": [2.35, 48.85, 2.35, 48.85], "features": [
  {"type": "Feature", "properties": {"name": "paris"}, "geometry": {"type": "Point", "coordinates": [2.35, 48.85]}},
  {"type": "Feature", "properties": {"name": "unknown"}, "geometry": null}]}

"geometry": null is what RFC 7946 says an unlocated feature is, so the row is kept and marked rather than dropped.

Three defects on one seam

float(None) raises TypeError, not ValueError. The existing except ValueError was already there to tolerate a bad coordinate, and it does catch float("abc"). But an empty cell arrives as None, not "", so it takes a different exception type and escapes the handler that exists to deal with exactly this.

'coordinates' in feature['geometry'] runs against a None. geometry_for_row returns None implicitly for a row with no coordinates, so the membership test raises TypeError: argument of type 'NoneType' is not iterable. --no-bbox skips this path entirely, which is why the shape can look like it works.

A bbox of nulls is not valid GeoJSON. With no usable coordinates anywhere, the output was "bbox": [null, null, null, null]. RFC 7946 requires bbox members to be numbers, and bbox itself is optional, so the right answer is to omit it.

The fix

Widen the existing catch to (TypeError, ValueError), guard the bbox accumulator with feature.get('geometry'), and add an is_set() check so the bbox key is only emitted when at least one coordinate was seen.

Rows with valid coordinates are unaffected, and so is --no-bbox.

Verification

Two cases in tests/test_utilities/test_csvjson.py with two small fixtures, one file mixing a good row with a blank one, one where every row is blank. They assert the bbox still covers the good row, that the blank row's geometry is null, and that no bbox key appears when nothing was located.

Reverting all three hunks fails both with the original TypeError: float() argument must be a string or a real number, not 'NoneType'. Reverting only the bbox hunk fails with 'bbox' unexpectedly found in {... 'bbox': [None, None, None, None] ...}, so each hunk is pinned separately.

pytest tests/test_utilities/test_csvjson.py is 26 passed. The full suite is 4 failed, 349 passed, and the same 4 csvstat locale failures occur on a clean tree here.

Catch TypeError in addition to ValueError when parsing coordinates, guard geometry membership test with feature.get(), and omit bbox when no coordinates are present. Fixes errors on rows with blank/unparseable lat/lon values.
geometry_for_row tested the coordinates for truthiness, so a latitude or longitude
of 0 was treated as absent. On master that produced a TypeError once the bbox
accumulator reached the None geometry; with the null-geometry handling in this branch
it would instead have silently written geometry: null for a valid location. The equator,
the prime meridian and null island are real coordinates, so test against None.
@VXNCXNX

VXNCXNX commented Aug 29, 2026

Copy link
Copy Markdown
Author

Self-review found a second defect on this same line, pushed as a follow-up commit.

geometry_for_row tested the coordinates for truthiness:

if lon and lat:

so a latitude or longitude of 0 counted as absent. That is the equator, the prime meridian and null island, all real locations.

On master this raises, because the bbox accumulator then walks into the None geometry:

$ printf 'name,lat,lon\ngreenwich,51.48,0\n' | csvjson --lat lat --lon lon
TypeError: argument of type 'NoneType' is not iterable

The first commit of this PR removed that crash, which would have turned it into something worse: a valid point silently written as "geometry": null. Loud wrong is better than quiet wrong, so this is the more important half of the fix.

Now lon is not None and lat is not None:

$ printf 'name,lat,lon\ngreenwich,51.48,0\n' | csvjson --lat lat --lon lon
{"type": "FeatureCollection", "bbox": [0.0, 51.48, 0.0, 51.48], "features": [
  {"type": "Feature", "properties": {"name": "greenwich"},
   "geometry": {"type": "Point", "coordinates": [0.0, 51.48]}}]}

Blank and unparseable values still produce "geometry": null as before.

Added examples/test_geo_zero.csv and test_geojson_zero_coordinates, covering all three zero shapes and asserting the bbox extends to 0. Restoring if lon and lat fails that test. Full suite: 350 passed, with the same 4 failures master already has here (test_csvstat, test_csvlook).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant