frictionlessdata / frictionless-r

R package to read and write Frictionless Data Packages
https://docs.ropensci.org/frictionless/
Other
29 stars 11 forks source link

`add_resource(replace = TRUE)` fails silently if resource does not exist #259

Open Rafnuss opened 3 months ago

Rafnuss commented 3 months ago

Hi, Is this desired? I would expect that replace=TRUE should not prevent the addition of a new (non-yet-exisiting) resource.

library(frictionless)

package <- example_package()

# Create a data frame
df <- data.frame(
  multimedia_id = c(
    "aed5fa71-3ed4-4284-a6ba-3550d1a4de8d",
    "da81a501-8236-4cbd-aa95-4bc4b10a05df"
  ),
  x = c(718, 748),
  y = c(860, 900)
)

# Add the resource "positions" from the data frame
add_resource(package, "positions", data = df)
#> A Data Package with 4 resources:
#> • deployments
#> • observations
#> • media
#> • positions
#> Use `unclass()` to print the Data Package as a list.

add_resource(package, "positions", data = df, replace = TRUE)
#> A Data Package with 3 resources:
#> • deployments
#> • observations
#> • media
#> Use `unclass()` to print the Data Package as a list.

I didn't look at the function, so not sure why this is happening.

peterdesmet commented 3 months ago

You didn't assign your first add_resource() to package (it is not updated in place). So the following works:

library(frictionless)
package <- example_package()

# Create a data frame
df <- data.frame(
  multimedia_id = c(
    "aed5fa71-3ed4-4284-a6ba-3550d1a4de8d",
    "da81a501-8236-4cbd-aa95-4bc4b10a05df"
  ),
  x = c(718, 748),
  y = c(860, 900)
)

package <- add_resource(package, "positions", data = df)

add_resource(package, "positions", data = df, replace = TRUE)
#> A Data Package with 4 resources:
#> • deployments
#> • observations
#> • media
#> • positions
#> Use `unclass()` to print the Data Package as a list.

Created on 2024-08-28 with reprex v2.1.0

However - and I'm not sure that this is what you wanted to point out - if you replace a resource that is not there, then nothing happens. It's probably better if:

  1. add_resource() just adds the resource then
  2. Or alternatively, add_resource() returns an error to say it can't find the resource

I prefer the latter.

Rafnuss commented 3 months ago

Yes, that's what I wanted to show (it works without Replace=T but fail with it).

Ok, I personally prefer the traditional overwrite=FALSE, which allows the modification even if the element does not exist. But up to you!

peterdesmet commented 3 months ago

Changed my mind, I agree that overwrite = TRUE should mainly be used to avoid overwriting resources by accident. It can silently add the resource if it isn't there yet.