WKTReader returns Envelope instead of Polygon

Question

WKTReader returns Envelope instead of Polygon

MoeweX opened this issue 6 years ago · 8 comments

When supplying a rectangle wktstring (POLYGON ((14 53, 13 53, 13 52, 14 52, 14 53))) to the WKTReader, the WKTReader returns an Envelope (ENVELOPE (13, 14, 53, 52)).

I find this non intuitive, as I would expect it to not change the "shape form". Is there a possibility to

prevent the WKTReader from "optimizing" the shape form?
or, to change the polygon shape "object" to an envelope shape "object" without readers/writers?

I guess this would also happen with other shapes, e.g., a circle, right?

Answer 1 · 2018-08-31T17:08:51.000Z

This choice is made here:
https://github.com/locationtech/spatial4j/blob/master/src/main/java/org/locationtech/spatial4j/io/WKTReader.java#L319
It seems this ought to be a configuration option of the parser, wether to call ShapeFactory.build() or ShapeFactory.buildOrRect()

Answer 2 · 2018-08-31T17:11:28.000Z

This only happens when parsing polygons.
As a work-around, you could extend this method of the parser to do what you want. The parser is deliberately extensible.

Answer 3 · 2018-08-31T17:19:10.000Z

What do you think about adding a configuration boolean to parseIfSupported() and parse()? This would then be supplied to parseShapeByType(). Or do you think the configuration should be done via the context factory that creates the parsers in the first place?

Answer 4 · 2018-08-31T17:26:32.000Z

What do you think about adding a configuration boolean to parseIfSupported() and parse()? This would then be supplied to parseShapeByType()

I think the proposed boolean, e.g. parsePolyAsRect, isn't so fundamental to parsing that it belongs in the method call to parse(). So yes I think it goes in the configuration of the context factory passed to the parser.

Answer 5 · 2018-09-01T00:32:04.000Z

This sounds like the default case should be to parse wkt strings without the transform to envelope functionality (as this it what a user expects: provide a polygon, get a polygon). I could create a pull request that removes the default transformation, but I do not want to add the configuration option that adds the transform functionality via the factory. What do you think?

…

Am 31.08.2018 um 19:26 schrieb David Smiley ***@***.***>: What do you think about adding a configuration boolean to parseIfSupported() and parse()? This would then be supplied to parseShapeByType() I think the proposed boolean, e.g. parsePolyAsRect, isn't so fundamental to parsing that it belongs in the method call to parse(). So yes I think it goes in the configuration of the context factory passed to the parser. — You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub, or mute the thread.

Answer 6 · 2018-09-01T02:48:19.000Z

I sympathize the default behavior is not correct and ought to be changed (and thanks for reporting it). But the main consumer/user of this API (Apache Lucene-spatial-extras / Solr) prefers an optimized representation of the shape for performance reasons. Are you objecting to this being a config toggle or just saying you don't want to do it?

Answer 7 · 2018-09-01T09:33:10.000Z

I meant the latter (I think adding a configuration option is a good idea), but if just changing the default is not an option because of Solr, I will for now rather overwrite the reader :).

Answer 8 · 2018-09-01T12:01:35.000Z

It’s ok to change a default. I just think it should be configurable.