Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Open
davidtheclark opened this issue Sep 30, 2016 · 5 comments
Open

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

davidtheclark opened this issue Sep 30, 2016 · 5 comments

Comments

@davidtheclark
Copy link
Contributor

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 ee8d854 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
Copy link
Contributor

tmcw commented Mar 20, 2017

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
Copy link
Contributor Author

I like the error better, too.

@andrewharvey
Copy link
Collaborator

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
Copy link

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

@Jmuccigr
Copy link

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.

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

No branches or pull requests

4 participants