R-ArcGIS / arcgislayers

Access ArcGIS Data and Location Services
http://r.esri.com/arcgislayers/
Apache License 2.0
44 stars 10 forks source link

Confusing error message for `arc_select()` when `...` contains invalid parameters #226

Open elipousson opened 1 week ago

elipousson commented 1 week ago

Describe the bug

The error message that results when ... contains invalid parameters is confusing. At a minimum, an improved error message should help with trouble-shooting – although other improvements to the validation of the ... argument may be helpful.

To Reproduce

library(arcgislayers)

furl <- paste0(
  "https://services3.arcgis.com/ZvidGQkLaDJxRSJ2/arcgis/rest",
  "/services/PLACES_LocalData_for_BetterHealth/FeatureServer/0"
)

flayer <- arc_open(furl)

# Invalid argument names are ignored if they contain strings
data <- arc_select(
  flayer,
  wher = "FALSE"
)
#> Iterating ■■■■                               9% | ETA: 12s
#> Iterating ■■■■■■■■■■■                       34% | ETA:  4s
#> Iterating ■■■■■■■■■■■■■■■■■■■■■■■■■■■■■■■  100% | ETA:  0s

# Invalid arguments give a confusing error if they contain something other than
# a string
data <- arc_select(
  flayer,
  wher = FALSE
)
#> Error in `arc_select()`:
#> ! `val` must be a single string, not `FALSE`.

Created on 2024-10-28 with reprex v2.1.1

Expected behavior

The error message should include the argument name of the invalid parameter to support trouble-shooting.

JosiahParry commented 1 week ago

The error messages appear to be due to the transition to the standalone checks.

Should be as simple as changing

-     check_string(val, allow_empty = FALSE)
+    check_string(val, arg = key, allow_empty = FALSE)
image
JosiahParry commented 1 week ago

Another challenge is keeping up with all of the possible parameters that can be passed in which are outlined at

https://developers.arcgis.com/rest/services-reference/enterprise/query-feature-service-layer/#request-parameters

These are different for AGOL and Enterprise based on version. So having beautiful validation might be out of the realm of possibility. But we can try?

Also, the way it works right now is that it accepts strings—since form bodies must be json encoded. We could allow anything that can be encoded into json using jsonify::to_json(x, unbox = TRUE)

elipousson commented 1 week ago

Something like this could allow additional queries to include logical and numeric values which might be useful.

# insert into query
for (i in seq_along(dots)) {
  key <- dots_names[i]
  val <- dots[[i]]

  if (is.logical(val)) {
    val <- tolower(val)
  } else if (!is.character(val)) {
    val <- as.character(val)
  }

  # check that the value is a scalar and non-empty
  check_string(val, arg = key, allow_empty = FALSE)

  # insert into query
  query[[key]] <- val
}
elipousson commented 1 week ago

It also might make sense to make sure dots_names does not contain "returnGeometry" since it will always be ignored in favor of the geometry argument or "outSR" since it is replaced by crs.