derhuerst / parse-gml-polygon

Convert a GML Polygon into GeoJSON.
https://github.com/derhuerst/parse-gml-polygon
ISC License
11 stars 2 forks source link

Parse SRSReferenceGroup/@srsDimension where applicable #10

Closed highsource closed 6 years ago

highsource commented 6 years ago

Many of the GML elements we process may contain the SRSReferenceGroup attribute group. This group contains coordinate system info in attributes like srsName and srsDimension.

Also from the spec:

If no srsName attribute is given, the CRS shall be specified as part of the larger context this geometry element is part of, e.g. a geometric aggregate.

Basically, nested elements should "inherit" srsName and srsDimension from outer elements. We do this via context, but at the moment only in PosList and Pos.

We should do it in other elements as well.

cyrilchapon commented 6 years ago

I also saw a srs format (dunno where it's documented) along with the coordinates, eg. in a bbox

minX,minY,maxX,maxY,srs-in-a-format-I-don't-remember
highsource commented 6 years ago

@cyrilchapon Never saw this...

highsource commented 6 years ago

@derhuerst I'm on it right now. Actually a peanuts but tests get a bit... cumbersome.

derhuerst commented 6 years ago

@highsource Do you mean because they're quite verbose? I think we can introduce some helper functions – but don't make the tests more complex than the code, that defeats their point.

highsource commented 6 years ago

I wrote a few tests to check that srsDimension propagates correctly via ctx/childCtx. Since srsDimension can be specified on almost any level, this makes combinatorics explode.

I'm not quite sure how to go about it.

derhuerst commented 6 years ago

I guess just have one test for propagation. I wouldn't aim for "test every edge case out there", but rather "test complex/likely edge cases and add more tests later".

cyrilchapon commented 6 years ago

@cyrilchapon Never saw this...

I'm hard looking for that example. I'll come back

derhuerst commented 6 years ago

@cyrilchapon I'm not saying your case is rare, I don't know if it is.

But the question regarding weird edge cases in general is how much we want to cover. As long as our library is easily adaptable, covering 99.9% of the edge cases won't be necessary.

highsource commented 6 years ago

@cyrilchapon Please don't get me wrong here, but I'm with @derhuerst. I think we should prioritize the cases. If something is too rare or not so common, I'd suggest skipping it for a moment. If someone needs it, they should at least supply the test case.

To be honest even the srsDimension handling I'm implementing at the moment looks not so common for me.

derhuerst commented 6 years ago

Closing as #12 is merged.