search

mapbox / csv2geojson

magically convert csv files to geojson files
http://mapbox.github.io/csv2geojson/
MIT License
362 stars 82 forks source link

Error or null geometry when lng,lat not detected? #50

Open davidtheclark opened 7 years ago

davidtheclark commented 7 years ago

In previous versions, this module threw an error when lng,lat were not detected. The documentation suggests this still should happen:

Err is non-falsy if latitude and longitude values cannot be detected or if there are invalid rows in the file.

However, I think commit https://github.com/mapbox/csv2geojson/commit/ee8d854540c5733bbc79c1f7678228988b850dc5 changed this behavior. The module now returns a null geometry. However, that commit did not update the documentation, and it left the relevant test in a strange place, where its description no longer matches its outcome.

So @tmcw I'm wondering which route you think would be best for this module. Should we document and clearly test the actual current behavior — not erroring, instead returning null geometry — or go back to the way things were — erroring?

tmcw commented 7 years ago

I preferred the error behavior, because it gave clear feedback to the user. null seems like silent failure, the worst kind. But apparently lots of people have CSVs with some but not all geodata, so maybe silent failure is what the people want.

davidtheclark commented 7 years ago

I like the error better, too.

andrewharvey commented 6 years ago

I prefer a warning. AS @tmcw points out lots of people have CSVs with some but not all geodata. This is all perfectly valid so I don't think csv2geojson shouldn't have an opinion about it, it should just pass it through.

If you error how would you use this tool to convert csv2geojson where some records have no geometry?

Jmuccigr commented 2 years ago

This is an old issue, but it seems that records with no geometry still get skipped. Is this to remain the behavior then?

Jmuccigr commented 2 years ago

Coming back to this again. It seems like the geojson spec 3.2 allows for null geometries when an object is unlocated, so empty lat and lon would seem then to call for that, no? I don't mind if an error is thrown as well, so long as those objects get included in the output geojson.