jimmyconti / nettopologysuite

Automatically exported from code.google.com/p/nettopologysuite
0 stars 0 forks source link

GeoJson: FeatureCollectionConverter does not parse any FeatureCollection #186

Closed GoogleCodeExporter closed 9 years ago

GoogleCodeExporter commented 9 years ago
What steps will reproduce the problem?
1.
var r = new GeoJsonReader();
var json = File.ReadAllText("geojson file"); // use example from 
https://en.wikipedia.org/wiki/GeoJSON
var fc = r.Read<FeatureCollection>(json); // fails

What is the expected output? What do you see instead?
A FeatureCollection is retrned

What version of the product are you using? On what operating system?
source from about March 2014, sorry do not know where I find the version

Please provide any additional information below.
This exception comes up:

Unhandled Exception: System.ArgumentException: Expected token 'features' not fou
nd.

The error is that the property

"type": "FeatureCollection",

is not parsed. The example file from wikipedia should be added as a test case 
for the GeoJson serializer.

Original issue reported on code.google.com by thu...@gmail.com on 21 May 2014 at 9:13

GoogleCodeExporter commented 9 years ago
as you can see with r1228, code works well if "type" json property is placed as 
last, but if is placed as first an error is thrown.
this is something I've experienced building topojson reader: there is some 
"constraints" that needs to be removed.

Original comment by diegogu...@gmail.com on 22 May 2014 at 3:23

GoogleCodeExporter commented 9 years ago
fixed with r1229: can you check the code?

Original comment by diegogu...@gmail.com on 22 May 2014 at 3:46

GoogleCodeExporter commented 9 years ago
the wikipedia example now runs, ignoring the order of properties is a good 
start. the way GeoJson is implementented is however still not the GeoJson 
specification. for instance GeoJson like this:

{
  "type": "FeatureCollection",
  "generator": "my-tool",
  "anotherproperty": "ccc",
 ...

fails. I fixed the GeoJson myself, in the FeatureConverterCollection I rewrote 
the code similar to yours, I also changed AttributesTableConverter and 
FeatureConverter. If you want I can send you the code. Even better would be to 
move this repo to github, than I could offer you a pull request.

Original comment by thu...@gmail.com on 23 May 2014 at 7:49

GoogleCodeExporter commented 9 years ago
custom properties others that crs, type and features aren't supported in the 
code. I can try to check geojson specs to see featurecollection specifications.

Original comment by diegogu...@gmail.com on 23 May 2014 at 8:05

GoogleCodeExporter commented 9 years ago
[deleted comment]
GoogleCodeExporter commented 9 years ago
From this:http://geojson.org/geojson-spec.html looks that featurecollection 
doesn't allow custom properties, so NTS behavior is correct: btw we can simply 
ignore custom properties, instead of throwing exceptions.
Anyway: GeoJSON code throws exceptions as design choice, so I'm eluctant to 
change this behavior only in FeatureCollectionConverter.

Original comment by diegogu...@gmail.com on 23 May 2014 at 8:09

GoogleCodeExporter commented 9 years ago
The spec might not be perfectly clear, but available APIs just append 
properties everywhere and they might be plain right:

http://wiki.geojson.org/GeoJSON_draft_version_6#FeatureCollection

Including additional members
GeoJSON allows additional members at any level in a GeoJSON object (as 
described in point 2 in the specification).

Original comment by thu...@gmail.com on 23 May 2014 at 10:36

GoogleCodeExporter commented 9 years ago
right. so it's something we should handle.

Original comment by diegogu...@gmail.com on 23 May 2014 at 10:54

GoogleCodeExporter commented 9 years ago
see r1230
now, for Featurecollections only, additional members are ignored.
two problems:
1. I don't have a place to push these additional members 
2. If additional members are complex objects, everything breaks
3. If you place an additional member or don't use the expected property order 
reading a Feature (as example) everything breaks.

To me looks that we need to check GEOJson stuff... but this looks a major 
rewrite, in a specific and different issue.

Any thoughts?

Original comment by diegogu...@gmail.com on 23 May 2014 at 11:04