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.

Sign up for GitHub

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

Jump to bottom

feat(api): add to_geo methods for writing geospatial output #10299

Merged
merged 1 commit into from
Oct 16, 2024
Merged

feat(api): add to_geo methods for writing geospatial output #10299

merged 1 commit into from
Oct 16, 2024

Conversation

cpcloud
Copy link
Member

@cpcloud cpcloud commented Oct 10, 2024 •
edited
Loading

Adds an experimental to_geo method to the duckdb backend.

Closes #10296.

@cpcloud cpcloud added this to the 10.0 milestone Oct 10, 2024
@cpcloud cpcloud added feature Features or general enhancements duckdb The DuckDB backend labels Oct 10, 2024
@github-actions github-actions bot added tests Issues or PRs related to tests dependencies Issues or PRs related to dependencies nix Issues or PRs related to nix labels Oct 10, 2024
@github-actions GitHub Actions
Copy link
Contributor

ACTION NEEDED

Ibis follows the Conventional Commits specification for release automation.

The PR title and description are used as the merge commit message.

Please update your PR title and description to match the specification.

@cpcloud cpcloud changed the title to geo feat(api): add to_geo methods for writing geospatial output Oct 10, 2024
gforsyth
gforsyth reviewed Oct 10, 2024
View reviewed changes
Copy link
Member

@gforsyth gforsyth left a comment

Choose a reason for hiding this comment

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

That windows exception is going to be hilarious to track down. I wonder which format is causing it to barf?

Overall I think this looks good -- should probably have a docstring with a link to the DuckDB geospatial page that lists all of the output options (and their names)

@cpcloud
Copy link
Member Author

cpcloud commented Oct 14, 2024 •
edited
Loading

DuckDB 1.1.2 was just released today (and I've merged a PR to update our lockfile to test against it). Let's see if that helps windows.

@cpcloud cpcloud force-pushed the to-geo branch 2 times, most recently from fa590fb to 1147168 Compare October 15, 2024 13:45
@github-actions github-actions bot added the ci Continuous Integration issues or PRs label Oct 15, 2024
@cpcloud cpcloud force-pushed the to-geo branch 2 times, most recently from 74e8dba to 42fa4a0 Compare October 15, 2024 15:00
@cpcloud
Copy link
Member Author

cpcloud commented Oct 15, 2024

Well, it turns out that it's the WAsP format, and I don't know why it crashes, so I just skipif'd it.

@cpcloud cpcloud force-pushed the to-geo branch 4 times, most recently from bd252f0 to dabd418 Compare October 16, 2024 11:15
@cpcloud cpcloud added the geospatial Geospatial related functionality label Oct 16, 2024
@cpcloud cpcloud requested a review from gforsyth October 16, 2024 11:29
gforsyth
gforsyth approved these changes Oct 16, 2024
View reviewed changes
Copy link
Member

@gforsyth gforsyth left a comment

Choose a reason for hiding this comment

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

:shipit:

@gforsyth gforsyth merged commit 9f565a9 into ibis-project:main Oct 16, 2024
77 checks passed
@cboettig
Copy link

🎉 🎉

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@gforsyth gforsyth gforsyth approved these changes

Assignees
No one assigned
Labels
ci Continuous Integration issues or PRs dependencies Issues or PRs related to dependencies duckdb The DuckDB backend feature Features or general enhancements geospatial Geospatial related functionality nix Issues or PRs related to nix tests Issues or PRs related to tests
Projects
None yet
Milestone
10.0
Development

Successfully merging this pull request may close these issues.

feat: add to_geo() method
3 participants
@cpcloud @cboettig @gforsyth