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

[SEDONA-498] Add ST_BestSRID #1246

Merged
merged 11 commits into from
Feb 22, 2024
Merged

[SEDONA-498] Add ST_BestSRID #1246

merged 11 commits into from
Feb 22, 2024

Conversation

prantogg
Copy link
Contributor

@prantogg prantogg commented Feb 20, 2024

Did you read the Contributor Guide?

Is this PR related to a JIRA ticket?

What changes were proposed in this PR?

This PR introduces ST_BestSRID, which estimates the most appropriate SRID for a given geometry, based on its spatial extent and location.

  • For geometries in the Arctic or Antarctic regions, the Lambert Azimuthal Equal Area projection is used.
  • For geometries that fit within a single UTM zone and do not cross the International Date Line, a corresponding UTM SRID is chosen.
  • In cases where none of the above conditions are met, or for geometries that cross the International Date Line, the function defaults to the Mercator projection.

How was this patch tested?

  • Passes new and existing tests

Did this PR include necessary documentation updates?

@prantogg prantogg marked this pull request as ready for review February 20, 2024 23:49
@jiayuasu
Copy link
Member

@prantogg Please add Snowflake doc

- In cases where none of the above conditions are met, or for geometries that cross the International Date Line, the function defaults to the Mercator projection.

!!!Warning
`ST_BestSRID` is designed to estimate a suitable SRID from a set of approximately 125 EPSG codes and works best for geometries that fit within the UTM zones. It should not be solely relied upon to determine the most accurate SRID, especially for specialized or high-precision spatial requirements.
Copy link
Contributor

Choose a reason for hiding this comment

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

I think this wouldn't render correctly as it is missing a tab.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Fixed. Good catch. Indentations don't seem to transfer when copy pasting. Thanks!

- In cases where none of the above conditions are met, or for geometries that cross the International Date Line, the function defaults to the Mercator projection.

!!!Warning
`ST_BestSRID` is designed to estimate a suitable SRID from a set of approximately 125 EPSG codes and works best for geometries that fit within the UTM zones. It should not be solely relied upon to determine the most accurate SRID, especially for specialized or high-precision spatial requirements.
Copy link
Contributor

Choose a reason for hiding this comment

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

Same thing here, tab missing.

Comment on lines 482 to 483
- For geometries that fit within a single UTM zone and do not cross the International Date Line, a corresponding UTM SRID is chosen.
- In cases where none of the above conditions are met, or for geometries that cross the International Date Line, the function defaults to the Mercator projection.
Copy link
Member

Choose a reason for hiding this comment

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

I've not found how the data line was handled in the implementation. Would you please point out where the date line was handled in bestSRID function?

Copy link
Contributor Author

@prantogg prantogg Feb 21, 2024

Choose a reason for hiding this comment

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

It does not handle geometries crossing the IDL appropriately. By the below line I mean - geometries crossing the IDL are mishandled and cause the determined SRID to default to Mercator projection SRID.
The mishandling happens with the angularWidth and angularHeight calculation; this causes the xwidth to be much larger that 6 degrees, which defaults the SRID to mercator.

In cases where none of the above conditions are met, or for geometries that cross the International Date Line, the function defaults to the Mercator projection.

This line is a bit confusing, let me clarify this in the doc

@jiayuasu
Copy link
Member

Snowflake functions of this PR are being tested in a separate PR: wherobots#11

@jiayuasu
Copy link
Member

Snowflake test failed: https://github.com/wherobots/sedona-snowflake-tester/actions/runs/7984544420/job/21801519880?pr=11


Since: `v1.6.0`

Spark SQL Example:
Copy link
Member

Choose a reason for hiding this comment

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

  1. Please call it SQL Example in all flink, spark, snowflake docs.
  2. Please say that this ST_BestSRID functions takes a WGS84 geometry as input and the geometry should follow lon/lat order.

@jiayuasu jiayuasu merged commit a8e529f into apache:master Feb 22, 2024
47 checks passed
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

4 participants