Skip to content

Get osmconf.ini file in synch with GDAL - #304

Merged
agila5 merged 14 commits into
masterfrom
synch_osmconf_ini
Apr 10, 2026
Merged

Get osmconf.ini file in synch with GDAL#304
agila5 merged 14 commits into
masterfrom
synch_osmconf_ini

Conversation

@agila5

@agila5 agila5 commented Nov 12, 2024

Copy link
Copy Markdown
Contributor

Fixes #261

  • Test on MacOS
  • Add tests
  • Add docs

agila5 and others added 6 commits November 12, 2024 16:13
Merge branch 'master' into synch_osmconf_ini

# Conflicts:
#	.github/workflows/R-CMD-check.yaml
Merge branch 'master' into synch_osmconf_ini

# Conflicts:
#	.github/workflows/R-CMD-check.yaml
skip ci
@agila5
agila5 requested a review from Copilot April 10, 2026 13:47
@agila5
agila5 marked this pull request as ready for review April 10, 2026 13:48

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

Updates how osmextract locates/uses osmconf.ini so the default GDAL OSM CONFIG stays aligned with the one shipped by the local GDAL/sf installation, with a package-bundled fallback; documentation and CI are updated accordingly.

Changes:

  • Add get_default_osmconf_ini() (exported) and update oe_vectortranslate() to use it as the default CONFIG source.
  • Replace hard-coded osmconf.ini line indices with heuristics to locate per-layer attributes= rows.
  • Update vignette/docs/NEWS and expand CI matrix to include macOS.

Reviewed changes

Copilot reviewed 8 out of 8 changed files in this pull request and generated 8 comments.

Show a summary per file
File Description
vignettes/osmextract.Rmd Updates vignette text/examples to reflect new default osmconf.ini discovery.
R/vectortranslate.R Implements default osmconf.ini discovery + new parsing helpers; wires into oe_vectortranslate().
R/get-key-values.R Switches default-field detection to use the newly detected default osmconf.ini.
NEWS.md Adds release note about synced CONFIG handling.
NAMESPACE Exports get_default_osmconf_ini().
man/oe_vectortranslate.Rd Regenerates docs for updated oe_vectortranslate() behavior.
man/get_default_osmconf_ini.Rd Adds generated man page for new exported function.
.github/workflows/R-CMD-check.yaml Expands checks to a macOS job and adjusts matrix strategy.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread R/vectortranslate.R
Comment on lines +491 to +509
# I cannot simply use grep(value = TRUE) since that matches the whole row, not
# only the part which I'm interested in. I need regexpr + regmatch
m = regexpr(
pattern = "(?<=^attributes=)\\S*",
text = file,
perl = TRUE
)
keys <- regmatches(x = file, m = m)
# The output of regmatches is a (character vector) which includes the matched
# substrings. It has a syntax like
# [1] a,b,c,d
# [2] a,d,e,f
# [3] b,f,g
# ...
# I need to split such sequence of keys using "," as a delimiter.
keys <- strsplit(keys, ",")
# I assume there are 5 layers specified according to the following order:
stopifnot(length(keys) == 5L)
stopifnot(layer %in% c("points", "lines", "multipolygons", "multilinestrings", "other_relations"))

Copilot AI Apr 10, 2026

Copy link

Choose a reason for hiding this comment

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

get_fields_default() currently uses regexpr() + regmatches() on the full file vector. For non-matching lines regmatches() returns empty strings, so keys <- strsplit(keys, ',') produces a list as long as file (not 5), making stopifnot(length(keys) == 5L) fail and breaking oe_vectortranslate() whenever extra_tags is used. Filter to only the matching attributes= lines (e.g., via grep('^attributes=', ..., value=TRUE) and sub('^attributes=', '', ...)) before splitting, or subset by m != -1L before calling regmatches()/strsplit().

Copilot uses AI. Check for mistakes.
Comment thread R/vectortranslate.R
Comment thread R/vectortranslate.R
Comment thread R/vectortranslate.R Outdated
Comment thread R/vectortranslate.R
Comment thread NEWS.md Outdated
Comment thread vignettes/osmextract.Rmd Outdated
Comment thread vignettes/osmextract.Rmd Outdated
agila5 and others added 3 commits April 10, 2026 15:57
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
@agila5
agila5 merged commit 42fd35c into master Apr 10, 2026
4 checks passed
@agila5
agila5 deleted the synch_osmconf_ini branch April 10, 2026 16:30
agila5 added a commit that referenced this pull request Apr 12, 2026
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.

[FEATURE] Get osmconf.ini in synch with GDAL

3 participants