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-703] Add utils for converting between RDD[Row] and SpatialRdd #1774

Closed
wants to merge 1 commit into from

Conversation

james-willis
Copy link
Contributor

@james-willis james-willis commented Jan 25, 2025

Did you read the Contributor Guide?

Is this PR related to a JIRA ticket?

What changes were proposed in this PR?

Added utils for converting between RDD[Row] and SpatialRDD. These are helpful for usecases that back physical nodes or functions that take in and return RDDs.

How was this patch tested?

Unit Tests

Did this PR include necessary documentation updates?

Docstrings are available.

@james-willis
Copy link
Contributor Author

@Kontinuation @jiayuasu is this logic duplicated anywhere else? Maybe somewhere in the Join logic?

@jiayuasu
Copy link
Member

@james-willis This is where Sedona join does the conversion:

DF to SpatialRDD:

def toSpatialRDD(rdd: RDD[UnsafeRow], shapeExpression: Expression): SpatialRDD[Geometry] = {

RDD to DF:

Please consider reusing the logic if possible,

@james-willis
Copy link
Contributor Author

Please consider reusing the logic if possible

Thanks Jia.

I was thinking that these functions I created would be useful for Physical Nodes but the linked code reminds me that InternalRows/UnsafeRows are used in these Physical Nodes, whereas the functions in this PR are for Rows. While you can convert between Rows and InternalRows, its an unnecessary conversion. It is probably best to keep distinct implementations for these two interfaces.

The fact that the implementations here are only useful for RDD[Row] but not RDD[InternalRow] makes me wonder if these are useful enough to introduce into the Sedona API.

@jiayuasu
Copy link
Member

jiayuasu commented Feb 4, 2025

Close in favor of #1780

@jiayuasu jiayuasu closed this Feb 4, 2025
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.

2 participants