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

feat(cli): Add support for geoparquet reading #247

Open
wants to merge 5 commits into
base: main
Choose a base branch
from

Conversation

alekzvik
Copy link

@alekzvik alekzvik commented Dec 20, 2024

Add support for reading geoparquet files in the CLI.

Closes #242

There is additional discussion and pointers in the linked ticket.

@alekzvik
Copy link
Author

Hello! I am new to rust and georust community and I did not find any contribution guidelines, so please let me know if anything can be improved.

Copy link
Member

@kylebarron kylebarron left a comment

Choose a reason for hiding this comment

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

This looks perfect! Indeed the RecordBatchReader is the right option; I should've suggested that previously.

I'm hoping to release geoarrow 0.4 within a couple weeks, but I don't expect it to change much from the latest beta, and certainly not any of these APIs.

Cargo.toml Outdated Show resolved Hide resolved
@@ -88,6 +91,26 @@ async fn transform<P: FeatureProcessor>(args: Cli, processor: &mut P) -> Result<
Some("jsonl") | Some("geojsonl") => {
GeozeroDatasource::process(&mut GeoJsonLineReader::new(filein), processor)
}
Some("parquet") | Some("geoparquet") => {
Copy link
Member

Choose a reason for hiding this comment

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

The community discourages against .geoparquet as a file extension, but I figure it's fine to accept files with the suffix.

Copy link
Author

Choose a reason for hiding this comment

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

I do not have a strong opinion here, and tried to be consistent with json/geojson and also with stacrs CLI which also supported both.

@kylebarron
Copy link
Member

kylebarron commented Dec 20, 2024

Note that due to compiler regressions: geoarrow/geoarrow-rs#716, rust-lang/rust#128887, rust-lang/rust#131960

this will fail to compile with cargo install (in release mode); I had to use

RUSTFLAGS="-Zinline-mir=no" cargo +nightly install --path .

to install from this branch

@kylebarron
Copy link
Member

geoarrow/geoarrow-rs#956 works around the compiler regression, so the next release of geoarrow will let you build this without the nightly API

@kylebarron
Copy link
Member

kylebarron commented Dec 20, 2024

I published geoarrow 0.4.0-beta.3 (https://crates.io/crates/geoarrow/0.4.0-beta.3) which lets you build without the nightly API. You can test out that beta release if you'd like.

I'll publish geoarrow 0.4 in early January. It might be simplest to wait to merge this PR until then

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.

Add geoparquet read support to geozero-cli
3 participants