IO GeoJSON Support

[michaelcurry]

@BenMorel : It looks like Geojson, KML, etc... is not currently supported. If a Pull Request came in that expanded the ability to Init and ToString multiple Geo String Types, would you be up for reviewing / merging?

[BenMorel]

Hi Michael, sure, I welcome pull requests!

This would fit very well as standalone reader/writer classes in the IO sub-namespace, unless you had something else in mind when you say Init / ToString?

[michaelcurry]

I have not dug in too much yet, but I am hoping to get some time this weekend.

P.S: Great Lib and thanks for writing it!

[michaelcurry]

@BenMorel : I 100% see what you mean by the IO sub-namespace 👍

Question:
The GeoJSON Spec (https://tools.ietf.org/html/rfc7946#section-3) specifies A GeoJSON object represents a Geometry, Feature, or collection of Features.. Because This Geo Lib does not account for "parameters" OR multiple Gemonstries from a single "read" Would you be ok with the parser only accepting GeoJSON Geometry?

NOTE: I could see an argument for accepting GeoJSON Geometry OR Feature as they only have one Geometry within them, but a Collection may hold multiple Geometry and the current Readers do not support Multiple Brick\Geo\Geometry returns. Example: https://github.com/brick/geo/blob/master/src/IO/WKTReader.php#L23

[michaelcurry]

@BenMorel Apologies for the long question... I think I just answered my own question.

It looks like the other Parsers throw a GeometryIOException if there is a "type" that is not supported (https://github.com/brick/geo/blob/master/src/Exception/GeometryIOException.php#L55)

Unless you specify otherwise, I will stick to this standard, and new "types" can be added to the parser as development continues. 👍

[michaelcurry]

@BenMorel: I have attached a Work In Progress Pull Request #14 for you to give feedback on before I take the time to complete the "Read" functionality for GeoJSON.

Any feedback would be great. Comments on the Pull Request itself might be the best way to remedy any issues.

[BenMorel]

Hi Michael,

A GeoJSON object represents a Geometry, Feature, or collection of Features.. Because This Geo Lib does not account for "parameters" OR multiple Gemonstries from a single "read" Would you be ok with the parser only accepting GeoJSON Geometry?

I've never used GeoJSON myself, but after a quick look, it looks to me that a GeoJSON object contains either a Feature, or a FeatureCollection (I've not found any mention of a "type": "Geometry" in the root GeoJSON object?); and that each Feature contains exactly one Geometry, plus arbitrary data (that we won't import here).

If this is correct, I would convert to GeoJSON object to:

• if it contains a Feature, to whatever Geometry the Feature contains
• if it contains a FeatureCollection, to a GeometryCollection that would be populated with each Geometry contained in each Feature the FeatureCollection contains

It looks like the other Parsers throw a GeometryIOException if there is a "type" that is not supported (https://github.com/brick/geo/blob/master/src/Exception/GeometryIOException.php#L55)

IIRC, this is when reading a WKT that would contain an unsupported type; this should not be necessary here, as all Geometry types in the GeoJSON spec are supported by this library!

[michaelcurry]

@BenMorel In reply to FeatureCollection:

The first version of the GeoJSON read, I don't plan to implement FeatureCollection type support, but After all of the types are complete, there may be an easy way to tack this on. 👍

[BenMorel]

I don't think this should be hard to add straight away, maybe it's worth thinking about it from the beginning?

Something like:

public function readGeometry(array $geoJsonObject) : Geometry
{
    switch (strtolower($geoJsonObject['type'])) {
        case 'feature':
            return $this->readGeometryFromFeature($geoJsonObject);

        case 'featurecollection':
            $geometries = [];

            foreach ($geoJsonObject['features'] as $feature) {
                $geometries[] = $this->readGeometryFromFeature($feature);
            }

            return GeometryCollection::of(...$geometries);
}

private function readGeometryFromFeature(array $feature) : Geometry
{
    $geometry = $feature['geometry'];

    switch (strtolower($geometry['type'])) {
        // ...
    }
}

[michaelcurry]

Good idea @BenMorel! I apparently overlooked the ability to return a GeometryCollection... Apologies; I have not used this Lib to date, but am excited to start using it in production once the GeoJSON support is complete.

Note: I have to run for a while but hope to continue working on the Pull Request later tonight/this weekend.

[michaelcurry]

@BenMorel: GeoJSON "read" functionality is complete. I am about to start on "write" functionality 👍

[michaelcurry]

@BenMorel: Pull Request is ready for you to look at 👍

I have to step out for a bit, but I will check any replies later tonight and make adjustments.
PR Link: #14

[michaelcurry]

First Pull Request Review Complete: (For Tracking)
526ed7f Remove GeoJSON spatial reference systems (SRIDs)
a3eda38 Add string $context to GeometryIOException::invalidGeoJSON(...)
9cfd97f Refactor GeoJSON Writer
651a5a4 Merge GeoJSON Abstract Reader into Reader class
52cf6bf Add GeoJSON json_decode checks in GeoJSONReader::read()
f58518c Return TypeHint GeoJSONWriter Private Functions
7ee7faa Change snake_case Param $geojson_array to camelCase $geojsonArray to follow codebase standards
0aeb6a4 Add GeoJSON genPoint ...$coods type as Float
48bd2fb Refactor Checking of GeoJSON FeatureCollection.Fetures array

[michaelcurry]

Pull Request update: All review items have been resolved. We should be going into final Tweaks 👍

[BenMorel]

Released as 0.2.2. Thank you!

[BenMorel]

Note: I added docs about import/export formats to the README, including GeoJSON!

[michaelcurry]

This is great @BenMorel
Thanks for getting this in. I am excited to extend the functionality in the near future.

[BenMorel]

I actually went ahead and added the case-insensitive read and pretty-print write functionality!
https://github.com/brick/geo/releases/tag/0.2.3

Please go ahead if you want to add KML support now 👍

[michaelcurry]

@BenMorel you beat me to it! Thanks for taking the time to do this.

I am actually looking into GeometryCollection GeoJson Geometry Objects. There is a change I will need to use this in a production use case, but I am not sure yet... Either way, It looks like I overlooked this Geometry Type when implementing the initial GeoJSONReader... Please stay tuned for that sometime this coming week... I hope...

[BenMorel]

Good point, I see that in the RFC. I created issue #15 to track this.