Repository navigation
Conversation
Some GDAL drivers, such as GeoPackage, report an FID column (e.g., rowid) that is virtual and not stored as an actual attribute field within the layer. This enhancement adds a user-friendly message to clarify this behavior, informing the user when the reported FID column is not among the layer's explicit fields. It also suggests a SQL-based workaround to expose the FID as an attribute, preventing potential confusion and improving usability.
Introduces a new module for GDAL vector schema management. This includes: - A canonical type map (`gdal_vector_type_map`) bridging OGR, SQL, Arrow, and R types. - An editable schema specification (`gdal_vector_schema_spec`) to define output fields (selection, renaming, retyping, FID handling, geometry). - Emitters to generate specific GDAL `vector pipeline` arguments (e.g., `select --fields`, `sql --sql`, `set-field-type`, `set-geom-type`). - A planning engine (`schema_spec_plan`) to determine the optimal transformation method. - A top-level function (`gdal_vector_schema_pipeline_args`) to automatically generate the minimal set of pipeline steps based on the user-defined schema spec. This significantly enhances control and automation for vector data processing workflows, ensuring consistent and correct schema application.
Introduces `cli_kv` for consistent key-value list formatting and `cli_json` for pretty-printed JSON output. These functions standardize the display of scalar fields and embedded metadata within `cli`-based `format`/`print` methods, ensuring a uniform user experience across the package.
Introduces a convenience wrapper around `base::readRenviron()` to simplify loading environment variables from a specified path. It provides user feedback using `cli::cli_alert_success()` upon successful loading.
Ensures package-specific cli styles (e.g., `.drv`, `.optname`, `.optval`) are available for use by output formatters across the package without needing to explicitly apply a local theme on every call. This centralizes theme management and promotes consistent styling.
Introduce a family of functions (`gpq_meta`) to read and parse GeoParquet (and plain Parquet) file metadata directly from the file footer. These functions provide structured access to: - File-level summary (`gpq_file_info`) - Parquet schema details (`gpq_schema_info`) - Row group and column chunk statistics (`gpq_row_groups`) - GeoParquet and GDAL specific metadata (`gpq_geo_metadata`) - Arrow-level schema (optional, `gpq_arrow_schema`) - A comprehensive aggregated view (`gpq_inspect`) Each function returns a classed list with `format()` and `print()` methods for user-friendly display, enhancing discoverability and understanding of file contents without loading data.
Validate the `gpq_meta` family of functions, ensuring correct parsing and reporting of GeoParquet and plain Parquet file metadata. This includes tests for `gpq_file_info`, `gpq_schema_info`, `gpq_row_groups`, `gpq_geo_metadata`, `gpq_arrow_schema`, and `gpq_inspect`. Includes new sample Parquet files (`geo.parquet`, `plain.parquet`) and a `test_data` helper function for consistent test data access.
…ion capabilities The new `gdal_vector_schema` functions (e.g., `gdal_vector_schema_spec`, `gdal_vector_schema_pipeline_args`) enable users to define, plan, and execute transformations on vector data schemas, including field selection, renaming, retyping, and geometry type adjustments. The `gpq_meta` family of functions (e.g., `gpq_inspect`, `gpq_file_info`, `gpq_arrow_schema`) are now fully exposed and documented, providing comprehensive tools to read and parse GeoParquet and plain Parquet file metadata directly from the footer without loading data. Also exports the `read_renviron` utility for convenient environment variable loading.
Provide a comprehensive set of geospatial data files (FGB, GeoParquet, PMTiles) for the Atlanta region. These files serve as examples and test cases for the recently introduced GeoParquet introspection and GDAL vector schema transformation capabilities.
This prevents local development artifacts or temporary files within the `context/` directory from being tracked by Git.
Leverages `gdalraster::srs_get_name()` and `srs_find_epsg()` to provide more robust and accurate CRS names and EPSG authorities from both PROJJSON and WKT strings within GeoParquet metadata. Updates the default `NULL` CRS handling to explicitly report OGC:CRS84, aligning with the GeoParquet specification. Extracts `.gpq_raw_to_int64` to a public `raw_to_int64` utility for wider use in decoding binary integer statistics. Refactors `cli_kv()` to use `cli::cli_verbatim()` with manual alignment and `cli::style_bold()` for keys, improving consistency and readability of key-value pair output. Introduces a new `is_blank` utility predicate for consistent handling of `NULL`, zero-length, or `NA` values.
Adjusts the default colors for `{.val}` and `{.cls}` spans to cyan, making them
more readable on dark terminal backgrounds. The theme is now applied once on
package load by layering onto the active cli theme, ensuring consistent
styling without modifying the user's session-wide cli configuration.
jimbrig
added
documentation
Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request introduces comprehensive support for GeoParquet metadata introspection and GDAL vector schema pipeline generation, including new modules R/gpq_meta.R and R/gdal_vector_schema.R, along with their corresponding documentation and tests. Feedback on these changes highlights several critical edge cases that could lead to runtime crashes or incorrect behavior. Specifically, the reviewer recommends checking for empty FID columns to prevent noisy warnings, validating the presence of geometry fields and key-value metadata to avoid out-of-bounds subscript errors, safely handling plain Parquet files that lack geospatial metadata, and gracefully handling zero-row Parquet files.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
| vec$getFIDColumn() | ||
| fields <- vec$getFieldNames() | ||
| gdal_fid_col <- vec$getFIDColumn() | ||
| if (!(gdal_fid_col %in% fields)) { |
There was a problem hiding this comment.
The FID column reported by GDAL can be an empty string "" for layers/drivers that do not have a FID column. Without checking nzchar(gdal_fid_col), this warning will be printed incorrectly for almost all non-FID layers. Adding a check for nzchar(gdal_fid_col) avoids this noisy and confusing warning.
if (nzchar(gdal_fid_col) && !(gdal_fid_col %in% fields)) {| rows <- c(list(fid_row), rows) | ||
| } | ||
|
|
||
| geom_field <- lyr$geometryFields[[1]] |
There was a problem hiding this comment.
For non-spatial tables or layers with no geometry fields, lyr$geometryFields will be an empty list. Attempting to access the first element with [[1]] will throw a subscript out of bounds error and crash the function. Checking the length of lyr$geometryFields first makes this safe.
geom_field <- if (length(lyr$geometryFields) > 0L) lyr$geometryFields[[1]] else NULL| check_file(gpq_path, ext = "parquet") | ||
|
|
||
| raw <- nanoparquet::read_parquet_schema(gpq_path) | ||
| geo_cols <- .gpq_read_meta(gpq_path)$geo$columns |
There was a problem hiding this comment.
For plain Parquet files (non-GeoParquet), .gpq_read_meta(gpq_path)$geo is NULL, which makes geo_cols NULL. Later on line 170, attempting to subset geo_cols[[nm]] will throw Error in geo_cols[[nm]] : object of type 'NULL' is not subsettable and crash the function. Defaulting to an empty list with %||% list() prevents this crash.
geo_cols <- .gpq_read_meta(gpq_path)$geo$columns %||% list()| #' @noRd | ||
| #' @importFrom tibble tibble | ||
| .gpq_kvm <- function(file_meta_data) { | ||
| kvm <- file_meta_data$key_value_metadata[[1L]] |
There was a problem hiding this comment.
If a Parquet file does not contain any key-value metadata, file_meta_data$key_value_metadata can be NULL or empty. Attempting to access [[1L]] directly will throw a subscript out of bounds or object of type 'NULL' is not subsettable error. Checking the length first ensures safety.
kvm <- if (length(file_meta_data$key_value_metadata) > 0L) file_meta_data$key_value_metadata[[1L]] else NULL
Comment on lines
+79
to
+80
| row_group_size = rg$num_rows[1L], | ||
| row_group_size_last = rg$num_rows[nrow(rg)], |
There was a problem hiding this comment.
If a Parquet file has 0 rows and 0 row groups, rg will have 0 rows. In this case, rg$num_rows[nrow(rg)] evaluates to rg$num_rows[0], which returns numeric(0). This results in a malformed list with a zero-length element. Checking nrow(rg) > 0L avoids this issue.
row_group_size = if (nrow(rg) > 0L) rg$num_rows[1L] else NA_integer_,
row_group_size_last = if (nrow(rg) > 0L) rg$num_rows[nrow(rg)] else NA_integer_,There was a problem hiding this comment.
Pull request overview
This PR (a "develop" merge) adds two substantial new capabilities to the gdalvector package: a GeoParquet metadata-inspection family (R/gpq_meta.R) that reads (Geo)Parquet footers via nanoparquet/arrow and exposes classed objects with format/print methods, and a GDAL vector schema-spec/pipeline-builder module (R/gdal_vector_schema.R) that emits gdal vector pipeline steps. It also adds supporting CLI/binary/predicate utilities, switches the package cli theme to be applied globally on load, and adds a FID-column diagnostic.
Changes:
- New
gpq_*GeoParquet metadata API + tests + roxygen docs, backed by newnanoparquet/arrowdependencies. - New
gdal_vector_schema_spec()/schema_spec_*/gdal_vector_schema_pipeline_args()schema-transformation engine + docs. - Utility additions (
raw_to_int64,cli_kv,cli_json,is_blank,read_renviron) and a globalcli.themeapplied inzzz.R; FID-column warning ingdal_vector_layer_fid_col().
Reviewed changes
Copilot reviewed 14 out of 35 changed files in this pull request and generated 5 comments.
Show a summary per file
| File | Description |
|---|---|
| R/gpq_meta.R | New GeoParquet footer-introspection function family with classed list outputs and formatters. |
| R/gdal_vector_schema.R | New schema-spec builder and pipeline/SQL emitters (no tests). |
| R/gdal_vector.R | Adds a diagnostic when GDAL's reported FID column isn't a real field; fires on empty FID name. |
| R/zzz.R | Applies the package cli theme globally via options(cli.theme=...) on load. |
| R/utils_cli.R | Adds cli_kv/cli_json renderers and brightened .val/.cls theme classes. |
| R/utils_binary.R | Adds raw_to_int64 for decoding 64-bit Parquet statistics. |
| R/utils_predicates.R | Adds is_blank helper. |
| R/utils_system.R | Documents/exports read_renviron. |
| DESCRIPTION | Adds arrow and nanoparquet to Imports. |
| NAMESPACE | Exports new functions, S3 methods, and importFrom entries. |
| man/*.Rd | Generated documentation for the new exported functions. |
| tests/testthat/test-gpq_meta.R, helper-data.R | Tests and helper for the new GeoParquet API. |
| .cursor/.gitignore | Ignores local context/ directory. |
Files not reviewed (16)
- man/gdal_vector_schema_pipeline_args.Rd: Generated file
- man/gdal_vector_schema_spec.Rd: Generated file
- man/gdal_vector_type_map.Rd: Generated file
- man/gpq_arrow_schema.Rd: Generated file
- man/gpq_file_info.Rd: Generated file
- man/gpq_geo_metadata.Rd: Generated file
- man/gpq_inspect.Rd: Generated file
- man/gpq_meta.Rd: Generated file
- man/gpq_row_groups.Rd: Generated file
- man/gpq_schema_info.Rd: Generated file
- man/read_renviron.Rd: Generated file
- man/schema_spec_arrow.Rd: Generated file
- man/schema_spec_plan.Rd: Generated file
- man/schema_spec_select_fields.Rd: Generated file
- man/schema_spec_set_field_type_args.Rd: Generated file
- man/schema_spec_set_geom_type_args.Rd: Generated file
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| Depends: | ||
| R (>= 4.2) | ||
| Imports: | ||
| arrow, |
| rlang::local_use_cli() | ||
| # layer the package's custom inline span styles (`.drv`, `.optname`, `.optval`, ...) onto the active cli theme | ||
| # so output formatters can use them without re-applying a local theme on every call. | ||
| options(cli.theme = utils::modifyList(getOption("cli.theme", default = list()), gdalvector_cli_theme())) |
| vec$getFIDColumn() | ||
| fields <- vec$getFieldNames() | ||
| gdal_fid_col <- vec$getFIDColumn() | ||
| if (!(gdal_fid_col %in% fields)) { |
| c( | ||
| "!" = "FID column reported by GDAL ({.field {gdal_fid_col}}) is not an actual field in the layer {.field {layer}}.", | ||
| "i" = "This is common for some drivers (e.g., GeoPackage) where the FID is a virtual column and not stored as a field.", | ||
| "i" = "Consider using {.field 'CAST(rowid AS INTEGER) AS source_fid'} in SQL performed against the layer get the FID used by GDAL as an attribute field" |
Comment on lines
+434
to
+445
| gdal_vector_schema_pipeline_args <- function( | ||
| spec, | ||
| layer, | ||
| where = NULL, | ||
| exclude_empty_geom = TRUE, | ||
| dialect = "SQLITE", | ||
| make_valid = FALSE, | ||
| set_field_type_policy = c("nonstring_keys", "all_nonstring", "all", "retyped"), | ||
| force_keys = character(), | ||
| multi = TRUE, | ||
| skip = TRUE | ||
| ) { |
…ilities This change removes the `gdal_vector_schema` module, including `gdal_vector_type_map()`, `gdal_vector_schema_spec()`, and `gdal_vector_schema_pipeline_args()` functions, along with their associated helpers. This streamlines the package by removing specific utilities for GDAL vector schema transformation that are no longer part of the intended core functionality. BREAKING CHANGE: Functions for GDAL vector schema specification and pipeline argument generation have been removed and are no longer available.
…edicate Rewrites `raw_to_int64()` to leverage the `bit64::integer64` class, ensuring more accurate and robust handling of 64-bit integers by correctly interpreting raw bytes. This replaces a less reliable manual implementation. The `is_blank()` utility predicate is removed as its functionality (`all(is.na())`) is straightforward and can be inlined directly, reducing unnecessary abstraction and simplifying the codebase.
The `cli` theme application is now handled by the new `gpq_cli_fmt()` helper, which wraps `cli::cli_fmt()` and applies the package's theme locally via `cli::cli_div()`. This prevents the theme from being applied globally, isolating package-specific styles and avoiding potential conflicts with other `cli`-enabled packages. Additionally, `cli_kv()` is refactored to use `cli::cli_bullets()` for rendering key-value pairs, improving consistency with `cli`'s idiomatic output. It now also supports an optional section title.
… Arrow dependency Splits `gpq_schema_info` into two functions: `gpq_schema` for the full column tibble and `gpq_schema_info` for a high-level summary. This provides a clearer API for inspecting Parquet schemas. Removes the `arrow` package from imports and the `gpq_arrow_schema()` function, streamlining dependencies. Simplifies CRS metadata summary by directly parsing embedded information instead of relying on GDAL's SRS API. Updates schema output to use `cli`'s bullet points and key-value pairs for better legibility. BREAKING CHANGE: The `gpq_arrow_schema()` function has been removed.
The test for `format` and `print` methods now explicitly asserts that `print()` emits its output invisibly, which is the idiomatic behavior for R print methods. Additionally, `expect_output()` is used to capture the printed panels, preventing them from cluttering the test console output.
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 11 out of 24 changed files in this pull request and generated 2 comments.
Files not reviewed (8)
- man/gpq_file_info.Rd: Generated file
- man/gpq_geo_metadata.Rd: Generated file
- man/gpq_inspect.Rd: Generated file
- man/gpq_meta.Rd: Generated file
- man/gpq_row_groups.Rd: Generated file
- man/gpq_schema.Rd: Generated file
- man/gpq_schema_info.Rd: Generated file
- man/read_renviron.Rd: Generated file
| vec$getFIDColumn() | ||
| fields <- vec$getFieldNames() | ||
| gdal_fid_col <- vec$getFIDColumn() | ||
| if (!(gdal_fid_col %in% fields)) { |
| c( | ||
| "!" = "FID column reported by GDAL ({.field {gdal_fid_col}}) is not an actual field in the layer {.field {layer}}.", | ||
| "i" = "This is common for some drivers (e.g., GeoPackage) where the FID is a virtual column and not stored as a field.", | ||
| "i" = "Consider using {.field 'CAST(rowid AS INTEGER) AS source_fid'} in SQL performed against the layer get the FID used by GDAL as an attribute field" |
Ensures that an empty FID column name reported by GDAL is explicitly rendered as `""` in the warning message, improving clarity for users. Switches from direct `cli::cli_bullets()` to `gdal_inform()` for consistent, categorized package messaging.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
No description provided.