Skip to content

GEOWAVE-398 & GEOWAVE-401. DWITHIN corrected to handle the unit conv…#403

Merged
rwgdrummer merged 1 commit intomasterfrom
GEOWAVE-398
May 28, 2015
Merged

GEOWAVE-398 & GEOWAVE-401. DWITHIN corrected to handle the unit conv…#403
rwgdrummer merged 1 commit intomasterfrom
GEOWAVE-398

Conversation

@rwgdrummer
Copy link
Copy Markdown
Contributor

…ersion properly from distance (meters, km, etc.) to decimal degrees (EPSG:4326 long/lat). There is still a degree of error difficult to compensate for when dealing polygons (e.g a country). Future work will adopt a more accurate approach consistent with other LocationTech projects

@coveralls
Copy link
Copy Markdown

Coverage Status

Coverage increased (+0.07%) to 62.69% when pulling df0d9a0 on GEOWAVE-398 into 441e24b on master.

1 similar comment
@coveralls
Copy link
Copy Markdown

Coverage Status

Coverage increased (+0.07%) to 62.69% when pulling df0d9a0 on GEOWAVE-398 into 441e24b on master.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

consider handling dateline crossing here

@rwgdrummer
Copy link
Copy Markdown
Contributor Author

Addressed one comment and left the other. Thinking better to have a false positive than a negative.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I think this really should cover us as much as we can for the dateline. In the last commit, this check was really all I was alluding to.

@coveralls
Copy link
Copy Markdown

Coverage Status

Coverage increased (+0.07%) to 62.68% when pulling 7b8ccbf on GEOWAVE-398 into 441e24b on master.

1 similar comment
@coveralls
Copy link
Copy Markdown

Coverage Status

Coverage increased (+0.07%) to 62.68% when pulling 7b8ccbf on GEOWAVE-398 into 441e24b on master.

@coveralls
Copy link
Copy Markdown

Coverage Status

Coverage increased (+0.07%) to 62.68% when pulling 7b8ccbf on GEOWAVE-398 into 441e24b on master.

@coveralls
Copy link
Copy Markdown

Coverage Status

Coverage increased (+0.07%) to 62.68% when pulling 7b8ccbf on GEOWAVE-398 into 441e24b on master.

@rwgdrummer rwgdrummer force-pushed the GEOWAVE-398 branch 2 times, most recently from ed53a3b to 9714f75 Compare May 27, 2015 15:13
@coveralls
Copy link
Copy Markdown

Coverage Status

Coverage increased (+0.19%) to 62.82% when pulling 9714f75 on GEOWAVE-398 into b7a6de2 on master.

@coveralls
Copy link
Copy Markdown

Coverage Status

Coverage increased (+0.07%) to 62.69% when pulling 9714f75 on GEOWAVE-398 into b7a6de2 on master.

@coveralls
Copy link
Copy Markdown

Coverage Status

Coverage increased (+0.15%) to 62.77% when pulling b16aff6 on GEOWAVE-398 into b7a6de2 on master.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

This method does not always give proper results. For example, it currently transforms long=116, lat=400 to long=116, lat=90 when the proper result should be long=116, lat=40.

Why do we call adjustCoordinateDimensionToRange() for the X coordinate but clipRange() for the Y and Z coordinates? Shouldn't all three coordinates rely on adjustCoordinateDimensionToRange()? It looks like the issues are stemming from clipRange()...

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

adjustCoordinateDimensionToRange() is just the wrong math to do with y and z - they don't have the same reflection property at the extrema.

90 + 1 degrees latitude is arguably 89 degrees instead of clamping at 90 but its definitely not -89 (you wouldn't want to shift from north to south pole)

…e unit conversion properly from distance (meters, km, etc.) to decimal degrees (EPSG:4326 long/lat). There is still a degree of error difficult to compensate for when dealing polygons (e.g a country). Future work will adopt a more accurate approach consistent with other LocationTech projects

Add 'some' support date line crossing for the DWITHIN function
Clip hemisphere crossing. A separate issue will be made for dealing the
poles.
rwgdrummer added a commit that referenced this pull request May 28, 2015
GEOWAVE-398 & GEOWAVE-401.  DWITHIN corrected to handle the unit conv…
@rwgdrummer rwgdrummer merged commit 8918fbf into master May 28, 2015
@chrisbennight chrisbennight deleted the GEOWAVE-398 branch June 27, 2015 15:15
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.

4 participants