Skip to content

Conversation

@yunchipang
Copy link
Contributor

@yunchipang yunchipang commented Oct 14, 2025

Did you read the Contributor Guide?

Is this PR related to a ticket?

What changes were proposed in this PR?

Implements union

How was this patch tested?

Tests added

Did this PR include necessary documentation updates?

  • Yes, I have updated the documentation.

Copy link
Collaborator

@petern48 petern48 left a comment

Choose a reason for hiding this comment

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

Thanks again @yunchipang! No extra feedback on this PR

@jiayuasu jiayuasu added this to the sedona-1.8.1 milestone Oct 15, 2025
@petern48
Copy link
Collaborator

One thing unrelated to the code changes itself. I would prefer for you to continue linking the GitHub issue (#2398) in the PR description. I don't personally care for anything else in there (maybe Jia will disagree), but linking the issue is useful because GitHub has a feature for auto-closing the issue when the PR is merged, when we link it. More importantly, if someone else stumbles across the issue, we want them to see your linked PR, so they don't try to duplicate your work before yours is merged.

extra nit: I also prefer that we mention Geopandas in the PR title because the title eventually becomes the commit message after merging. (don't worry about your previous PRs). It's more descriptive in cases where anyone looks back through commit history or PR history (for debugging, for example). e.g. something like "Geopandas: Implement union" or "Implement geopandas union"

@jiayuasu jiayuasu linked an issue Oct 15, 2025 that may be closed by this pull request
@yunchipang yunchipang changed the title [GH-2398] Implement union [GH-2398] Implement geopandas union Oct 15, 2025
@yunchipang
Copy link
Contributor Author

@petern48 yep sure will do. i did forget to tag issue number in this PR (sorry!) but the previous ones are all tagged. will make sure geopandas is mentioned in PR title moving forward as well. thanks for the heads up!

@jiayuasu jiayuasu merged commit 073488d into apache:master Oct 15, 2025
32 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.

Geopandas: Implement union

3 participants