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

Feature/fix config parameter #33

Open
wants to merge 4 commits into
base: master
Choose a base branch
from

Conversation

mwip
Copy link

@mwip mwip commented May 23, 2020

This PR closes #30

It introduces a parameter_named to gdal_cmd_builder() which allows to parse options built on key value pairs. It is specifically designed to support adding e.g. --config GDAL_CACHEMAX "30%" or alike to gdal commands where suitable.

I would kindly like to ask @rolfsimoes for a code review.

mwip added 3 commits May 23, 2020 23:04
Previously, --config parameters were parsed as 'normal' doubledashed
parameters in gdal_cmd_builder. However, this --config options must be
provided as key value pairs like `gdalinfo /some/image.tif --config
GDAL_CACHEMAX "30%" for setting the maximum cache to 30 percent of the
system memory.

This commit introduces changes to the creation of the gdal commands in
gdal_cmd_builder.R to introduce the general functionality. Further, all
functions which already incorporate a config parameter were adjusted to
make use of the new functionality.

In the case of gdalinfo the "config" parameter was moved from character
parameters to repeatable parameters, which is in line with the handeling
in all other functions.
The documentation of the newely added parameter_named was adapted.
Also in the documentation in all functions having a config parameter
were the config parameter was changed to a named character.
After checks, devtools::document() was used to update the documentation.
The package builds and tests without any errors, warnings and notes.
Copy link

@rolfsimoes rolfsimoes left a comment

Choose a reason for hiding this comment

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

Minor changes.
Nice work!

R/gdal_cmd_builder.R Outdated Show resolved Hide resolved
R/gdal_cmd_builder.R Outdated Show resolved Hide resolved
R/gdal_cmd_builder.R Show resolved Hide resolved
@mwip
Copy link
Author

mwip commented Jun 11, 2020

@rolfsimoes Thanks for your help. I resolved the few comments.
@jgrn307, this should wrap up the PR.

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.

Fix --config parameter
2 participants