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

WKTReader returns Envelope instead of Polygon #166

Open
MoeweX opened this issue Aug 31, 2018 · 8 comments
Open

WKTReader returns Envelope instead of Polygon #166

MoeweX opened this issue Aug 31, 2018 · 8 comments

Comments

@MoeweX
Copy link
Contributor

MoeweX commented Aug 31, 2018

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?

@dsmiley
Copy link
Contributor

dsmiley commented Aug 31, 2018

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()

@dsmiley
Copy link
Contributor

dsmiley commented Aug 31, 2018

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.

@MoeweX
Copy link
Contributor Author

MoeweX commented Aug 31, 2018

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?

@dsmiley
Copy link
Contributor

dsmiley commented Aug 31, 2018

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.

@MoeweX
Copy link
Contributor Author

MoeweX commented Sep 1, 2018 via email

@dsmiley
Copy link
Contributor

dsmiley commented Sep 1, 2018

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?

@MoeweX
Copy link
Contributor Author

MoeweX commented Sep 1, 2018

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 :).

@dsmiley
Copy link
Contributor

dsmiley commented Sep 1, 2018

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants