cynkra / dm

Working with relational data models in R
https://dm.cynkra.com
Other
502 stars 49 forks source link

`as_dm()` accepts non-dataframe objects which can lead to unexpected errors #2197

Open DanChaltiel opened 9 months ago

DanChaltiel commented 9 months ago

Hi,

I recently discovered this great package, thank you very much for your work!

I'm working with a package that outputs a list of dataframe from a database. However, the list also contains some metadata members of various types along with the dataframes. Using as_dm() on the list throws no warning regarding those members and their presence causes some bugs in functions such as dm_draw().

Here is a reprex:

library(dm)
a = list(a=iris, b=mtcars, x=1)
x = as_dm(a)
x
#> -- Metadata --------------------------------------------------------------------
#> Tables: `a`, `b`, `x`
#> Columns: 16
#> Primary keys: 0
#> Foreign keys: 0

dm_draw(x, rankdir = "TB", view_type = "all")
#> Error in `vec_rbind()`:
#> ! Can't combine `..1$column` <character> and `..3$column` <logical>.

Created on 2024-02-21 with reprex v2.0.2

Note that x is considered as a table in the printed Metadata.

The error points to unnest.R#L47, and is caused by non-dataframes yielding a logical (and empty) column column in col_data.

I would expect either an early error in as_dm() forbidding the list to contain any non-dataframe, or an automatic removal of non-dataframes, maybe with a warning. A hint to use purr::keep(my_list, is.data.frame) might be useful too.

I'd be happy to provide a PR if that can help you.

krlmlr commented 2 days ago

Thanks, I agree we should be testing inputs more carefully here. Would you like to contribute?