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

Issue2395 #35

Merged
merged 19 commits into from
Feb 4, 2013
Merged

Issue2395 #35

merged 19 commits into from
Feb 4, 2013

Conversation

mpazos
Copy link
Contributor

@mpazos mpazos commented Jan 25, 2013

This pull request solves the issue

http://applis-bretagne.fr/redmine/issues/2395

An additional file which contains the bbox is created taking into account the format file selected (shp, tab, mif/mid)

};
LinearRing shell = geomFactory.createLinearRing(coordinates);

Polygon geometry = new Polygon(shell, new LinearRing[]{} , geomFactory);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

there is an easier way to convert a bbox to a geometry. You can do: GeometryFactory.toGeometry(evelope)

I don't remember the exact method name but it is something like that. I won't hold up this request for the change but I would like a second pull request with this change.

unless of course you know of a reason not to use that method

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I have found this method in JTS
JTS.toGeometry(bbox);

so I could replace that code by

BoundingBox bbox = envelope.toBounds(targetCrs);
Polygon polygon = JTS.toGeometry(bbox);
(The transformation is managed by the toBound method)

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

that would work

@jesseeichar
Copy link
Member

I am going to merge this change. But could you make a second pull request that fixes the two comments I made?

jesseeichar pushed a commit that referenced this pull request Feb 4, 2013
@jesseeichar jesseeichar merged commit 06e2528 into georchestra:master Feb 4, 2013
@mpazos
Copy link
Contributor Author

mpazos commented Feb 4, 2013

Ok, I will include your advices in a new pull request.

@mpazos
Copy link
Contributor Author

mpazos commented Feb 4, 2013

done!

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

Successfully merging this pull request may close these issues.

2 participants