Skip to content

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

Open
VXNCXNX wants to merge 1 commit 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 1 commit 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.
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