Add set_layer_coded_values() and export set_layer_col_names()

[elipousson]

Checklist

• update NEWS.md
• documentation updated with devtools::document()
• devtools::check() passes locally

Changes

• Export set_layer_col_names() (previously used internally by arc_read()) to allow use of alias values with data returned by arc_select() (Support use of field alias values as column names or label attributes by arc_read() #169).
• Add new set_layer_coded_values() function to support replacement or labelling of values with coded value domains (Replace domains information #134).

Also add helper functions to support set_layer_coded_values():

• list_field_domains()
• pull_pull_coded_values()

Issues that this closes

Partly addresses this issue: #134

There may be a need for functions addressing the other type of domain values and/or validating data for consistency with set domain values before adding data to an existing layer.

Follow up tasks

• Add tests for set_layer_coded_values()
• Document and export list_field_domains()

[JosiahParry]

This is looking pretty good! Thank you! There's a few changes I think we can take to simplify the behavior.

set_layer_col_names()

1. We should rename set_layer_col_names() I think it is a bit too verbose and the col I think is unnecessary because, for better or worse, no one uses row names.
   • alternative names could beset_layer_names(), rename_layer_cols()
2. The col_names argument should be removed. The inclusion of this to me seems like it is smooshingdplyr::rename or rlang::set_names() into the same function. Doing so makes the function not have a single purpose but two. I would prefer that this function is designed to specifically rename based on the upstream layer

set_layer_coded_values()

This function seems much more straight forward. I do think that it needs a better name, though. Perhaps we can call it something like encode_from_domains() or encode_fields() the idea being that the function is more directly connected to the action being taken on the users behalf.

Testing

Lastly, can we add a few tests for naming? We can create some mocked layer objects too. I'm happy to help there.

[JosiahParry]

Let's check for null nm here and error out safely

[elipousson]

Took more work than I hoped to disentangle the col_names argument handling from the alias handling but I think you're right and it is better this way. Proposing set_layer_aliases and encode_field_values as the new function names. If you wanted to add some tests, that would be a big help.

On reflection, I also don't think list_field_domains() really need to be exported - it seems unlikely anyone who isn't a developer would need it.

[JosiahParry]

@elipousson arc_read() is now broken because the default argument in arc_read() is drop.

How do you want to handle this? Ideally, the default behavior is do nothing.

furl <- "[https://sampleserver6.arcgisonline.com/arcgis/rest/services/Census/MapServer/3](vscode-file://vscode-app/Applications/Positron.app/Contents/Resources/app/out/vs/code/electron-sandbox/workbench/workbench.html#)"
res <- arc_read(furl, n_max = 20)
#> Error in `arc_read()`:
#> ! `alias` must be one of "label" or "replace", not "drop".

Relatedly it looks like arc_read(furl, n_max = 20, col_names = FALSE) creates an error because the geometry column name is renamed without setting the attr(x, "sf_column") appropriately.

We should probably figure this out within this PR too—scope creep eeek!—so that we're not merging any more bugs.

Looks like the logic in set_col_names() is a bit out of order. I also think there may be some duplication or a bit of obfuscation with the repair_layer_names() and set_col_names() which might make catching these issues a bit tougher.

For example set_col_names() is defined and used only once inside of arc_read() and repair_layer_names() is used inside of set_col_names() and set_layer_aliases(). These functions are both modifying the column names and it feels a bit redundant / cluttered. Though I'm not quite sure how to tidy it up.

[JosiahParry]

@elipousson are you okay if i take a stab at this a bit?

[elipousson]

I know exactly how the bugs got introduced - I tried to account for both of them but clearly didn’t. To fix the first bug, you can validate alias with arg_match inside of arc_read before calling set_col_names. Have at it if you like or I can get to it tonight or tomorrow!

[elipousson]

Got it!

[JosiahParry]

I've gone ahead and taken the liberty to finish this PR. Note that all exported functions must have an example and also specify the @returns tag otherwise CRAN maintainers can reject the package.

• I've added argument validation to set_layer_aliases(), encode_field_values(), and arc_read()
• I've changed the default from "label" to replace for set_layer_aliases()—I'm still unsure what the label attribute gives you. This seems useful for advanced users who may want to play with attributes.
• I've added tests for encode_field_values()
• I've added tests for set_layer_aliases()
• I've added tests for arc_read()