frictionlessdata / frictionless-r

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

`append()` drops custom `datapackage` class #198

Open peterdesmet opened 7 months ago

peterdesmet commented 7 months ago

When using append(), the datapackage lists looses its custom class:

library(frictionless)
p <- create_package()
class(p)
#> [1] "datapackage" "list"
p <- append(p, c(foo = "bar"))
class(p)
#> [1] "list"

Created on 2024-03-27 with reprex v2.1.0

This results in check_package() warnings like in #194. I assume the way to solve this is by creating a append.datapackage() function.

To update:

PietrH commented 4 months ago

I'm struggeling because append() is not an S3 generic:

> sloop::is_s3_generic("append")
[1] FALSE

Thus creating a method for it is not as strait forward as it was for print()

PietrH commented 4 months ago

The answer might be here: https://stat.ethz.ch/R-manual/R-devel/library/methods/html/Methods_for_Nongenerics.html

Or here: https://stackoverflow.com/questions/66811606/how-to-override-the-implementation-of-a-non-generic-function-in-r

PietrH commented 4 months ago

Something something methods::setGeneric() ?

peterdesmet commented 4 months ago

@khusmann would you know how to extend base R append() for a specific S3 class? Cf. print() we would like to avoid a masking warning when loading the package.

khusmann commented 4 months ago

I don't think there's any good way of doing this. The standard way of doing this, from what I've seen (and what @PietrH linked), is to alias/mask the method, and make the default S3 call the base implementation of the method. The problem with this, I think, is that packages that depend on frictionless-r would all have to re-import that S3 modified version of append in order for it to work properly. (See the tidyverse generics package for example)

My recommendation is to instead build an API around datapackage manipulations that provides pipe-able verbs with a common prefix for building / modifying datapackages, specifically for datapackages. So provide dpkg_append, or dpkg_mutate, etc., In addition to preserving the class, it would also give you the ability to validate the descriptor as modifications were made, if desired.

I think it's a big open question though what base verbs/operations would form an optimal expressive api here... If you ever have a brainstorming session, i'd definitely be interested in participating!