Skip to content

Expose Spatial Join through CLI command#1319

Merged
rfecher merged 1 commit intolocationtech:masterfrom
JWileczek:join-final
Apr 10, 2018
Merged

Expose Spatial Join through CLI command#1319
rfecher merged 1 commit intolocationtech:masterfrom
JWileczek:join-final

Conversation

@JWileczek
Copy link
Copy Markdown
Contributor

Work to expose spatial join directly through CLI. Updates algorithm to be run primarily using SpatialJoinRunner. There are only a couple areas of the code I don't feel entirely comfortable with:

  1. The code surrounding the creation of the new data adapter for the join results particularly the naming of the new adapter. If there is a better way to come up with a unique adapter Id for the join that follows other areas where unique names are needed I would rather switch my code to follow that pattern.
  2. The UDF registry class for spark udf functions. I hacked a bit for the WKT functions which is fine in my mind since that will be removed eventually, but the part that bothers me is a rather small switch but the change from 'g' to 'G' for the registered udf name in Spark. Technically this is a complete API change even though it is super tiny so it will break any older versions that were using those UDF functions. Should we keep maintaining those previous names? I chose not to in this PR because it adds a bunch of unnecessary code, but I can add them back in if we should keep supporting them.

@JWileczek JWileczek requested a review from rfecher April 9, 2018 20:07
Copy link
Copy Markdown
Contributor

@rfecher rfecher left a comment

Choose a reason for hiding this comment

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

looks good to me, just waiting to see it pass

@rfecher rfecher merged commit 10923e9 into locationtech:master Apr 10, 2018
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