Properties, CRS and other optional geo_interface items
jzmiller1 opened this issue · 10 comments
I might have overlooked it but it seems like the as_shape function and geometry classes don't take advantage of any of the optional geo_interface keys (and that the geo_interface specification itself omits the crs key in the GeoJSON standard). Could you provide some detail on why these were omitted? Are these things that you felt that pygeoif users should implement themselves?
Would pull requests that implemented them be looked upon favorably or at least be something you would be interested in reviewing and discussing?
Thanks for your work on pygeoif!
Sorry for answering so late, I have not noticed it.
It is designed after https://gist.github.com/sgillies/2217756 which does not mention the crs.
If you'd like to add it and it does not break anything that would be fine with me. Thanks for bringing this up :-)
If I may join the conversation on this, I wrote a slightly nonstandard Feature class for Total Open Station: https://github.com/steko/totalopenstation/blob/3d61f426278b867c5845e92056861c94ab635772/totalopenstation/formats/__init__.py#L33
This was even before pygeoif had a Feature class of its own (actually, both were done on 4th March, go figure! 😀 ). Now I'd like to stick with the pygeoif version, but can we discuss the possibility of having a top-level key id that is not nested in properties, in line with the the GeoJSON spec for Feature objects? If there is agreement on this, I could submit a pull request. Otherwise I will happily subclass from pygeoif.Feature.
Thanks for creating pygeoif and for keeping it alive with contributions from others.
I searched through the GeoJSON spec at GeoJSON.org and the Feature seems to
be the only object with this top level ID as an option. Did you notice it
anywhere else?
On Aug 1, 2015 5:27 AM, "Stefano Costa" notifications@github.com wrote:
If I may join the conversation on this, I wrote a slightly nonstandard
Feature class for Total Open Station:
https://github.com/steko/totalopenstation/blob/3d61f426278b867c5845e92056861c94ab635772/totalopenstation/formats/__init__.py#L33This was even before pygeoif had a Feature class of its own (actually,
both were done on 4th March, go figure! [image: 😀] ). Now I'd
like to stick with the pygeoif version, but can we discuss the possibility
of having a top-level key id that is not nested in properties, in line
with the the GeoJSON spec for Feature objects? If there is agreement on
this, I could submit a pull request. Otherwise I will happily subclass from
pygeoif.Feature.Thanks for creating pygeoif and for keeping it alive with contributions
from others.—
Reply to this email directly or view it on GitHub
#4 (comment).
Yes, it's only allowed for Feature objects (my wording was not clear in the previous message).
Yes, that's what I was thinking of, thanks!
I only have one question: why did you make _feature_id private instead of choosing feature_id? Alternatively, the shorter fid could have been used, too (not saying this short would be the best, just asking).
I made _feature_id private because _geometry and _properties have been private since before I started looking at this project. I just wanted to blend in with what was already going on. I should have added a _feature_id = None to the class to match the _properties and _geometry but I just noticed that I missed it.
I preferred to use just id as the variable name but that collides with some python built in stuff as far as I can tell. I used feature_id over fid as it makes the code more readable to me.
Right, I see the rationale and I agree. As you say, there should be a _feature_id = None and, I think, also a @property as for _properties and _geometry.
Looks OK.