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

Remove LineString upcast when accessing Polygon rings #290

Merged
merged 2 commits into from
Nov 21, 2018

Conversation

dbaston
Copy link
Contributor

@dbaston dbaston commented Jun 12, 2018

This makes it simpler for a user to construct new Polygons from one or more rings of an existing Polygon.

Do this to make it simpler for a user to construct new Polygons from one
or more rings of an existing Polygon.

Signed-off-by: Daniel Baston <dbaston@gmail.com>
@dr-jts
Copy link
Contributor

dr-jts commented Jun 12, 2018

The use of LineString as the return type was done to follow the OGC Simple Features Specification (see OGC SFS 1.1.0 Section 6.1.11.2). Not sure why they used LineString rather than LinearRing.

Any comments on whether it's better to be spec-compliant, or easier to use?

@dbaston
Copy link
Contributor Author

dbaston commented Jun 12, 2018

Hmm, I figured as much. I'll weasel around the tough question and make the argument that the PR is spec-compliant, because a LinearRing is indeed a LineString.

@dr-jts
Copy link
Contributor

dr-jts commented Jun 12, 2018

That seems reasonable to me. Still odd that they didn't do this in the spec - but I can't see a reason for this, so perhaps it just slipped through the cracks (it's the kind of thing that might not be noticed until people actually started using the spec).

Signed-off-by: Daniel Baston <dbaston@gmail.com>
@dbaston
Copy link
Contributor Author

dbaston commented Nov 21, 2018

@dr-jts can this be merged, or should I close it out?

@dr-jts dr-jts merged commit 8ba2900 into locationtech:master Nov 21, 2018
@jodygarnett
Copy link
Contributor

This has broken geotools build:

@jodygarnett
Copy link
Contributor

jodygarnett commented Feb 15, 2019

Looking at failure in downstream application:

Exception in thread "SunTileScheduler0Standard1" java.lang.NoSuchMethodError: org.locationtech.jts.geom.Polygon.getExteriorRing()Lorg/locationtech/jts/geom/LineString;
	at it.geosolutions.jaiext.utilities.shape.PolygonIterator.<init>(PolygonIterator.java:61)
	at it.geosolutions.jaiext.utilities.shape.GeomCollectionIterator.getIterator(GeomCollectionIterator.java:92)
	at it.geosolutions.jaiext.utilities.shape.GeomCollectionIterator.init(GeomCollectionIterator.java:65)
	at it.geosolutions.jaiext.utilities.shape.GeomCollectionIterator.<init>(GeomCollectionIterator.java:75)
	at it.geosolutions.jaiext.utilities.shape.LiteShape.getPathIterator(LiteShape.java:245)

It appears that the exact method signature has been baked into the resulting byte code as if this was a final method.

Some research is needed to see if narrowing the return type is a breaking change in java.

@jodygarnett
Copy link
Contributor

Okay because this was an API change we should of updated the version number to 1.17.0-SNAPSHOT.

I am going to see if I can pull over everything except this to the 1.16.x branch

@dr-jts
Copy link
Contributor

dr-jts commented Feb 16, 2019

@jodygarnett There have been a lot of changes since 1.16. To cut a 1.16.1 should probably just pull over the commits fixing CoordSequences and leave it at that.

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

Successfully merging this pull request may close these issues.

None yet

3 participants