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

Moved 'properties' up the GeoJSON class tree #1

Merged
merged 1 commit into from
Oct 5, 2013
Merged

Moved 'properties' up the GeoJSON class tree #1

merged 1 commit into from
Oct 5, 2013

Conversation

JosephCottam
Copy link
Contributor

My reading of section 1 GeoJSON (http://geojson.org/geojson-spec.html#introduction) spec says that properties can be held by most any type of data. Not having the properties prevented deserialization of US counties found in the world.geo.json collection (https://github.com/johan/world.geo.json).

This pull request moves the properties dictionary up to a higher point in the class tree. It also adds an annotation to prevent serialization of the properties dictionary when there are none (thus, tests did not need to change).

@grundid
Copy link
Member

grundid commented Oct 4, 2013

Thanks for your pull request. Looking at the geojson spec I thought that only Features can have properties.
"Features in GeoJSON contain a geometry object and additional properties".

Could you please post the example that cannot be parsed without the properties?

@JosephCottam
Copy link
Contributor Author

As I read the spec, you are only required to handle "properties" on a "Feature" object, but they are not forbidden elsewhere.

Cannot be parsed without properties (does not preclude the below being improper, but its one of the things I was working with): https://raw.github.com/johan/world.geo.json/master/countries/USA/AK/Aleutians%20East.geo.json

The solution I provided above specifically promotes properties up so they would still be accessible if found. An alternative would be to encode a more robust parser that generally ignores non-required items.

grundid added a commit that referenced this pull request Oct 5, 2013
Moved 'properties' up the GeoJSON class tree
@grundid grundid merged commit bc00dcc into opendatalab-de:master Oct 5, 2013
@grundid
Copy link
Member

grundid commented Oct 5, 2013

Thanks again. Your contribution will be part of release 1.1 that I'm pushing right now to maven central. It should be available for download within a few hours.

grundid pushed a commit that referenced this pull request Nov 19, 2015
Update <version> to 1.5.1 in README.md
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.

None yet

2 participants