cynkra / dm

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

additions to `dm_flatten_to_tbl()` #65

Open TSchiefer opened 5 years ago

TSchiefer commented 5 years ago

IMHO cdm_flatten_to_tbl() should feature:

  1. a logical parameter drop_key_cols, since we often do not want to keep the code-columns used as keys, but only the real info (then reunite_parent_child() might come in handy)
  2. the user should be informed by a message which tables are joined to the start-table
krlmlr commented 5 years ago
  1. Maybe we could support %>% cdm_select(table, -is_pk()) instead?
  2. Good idea -- unless given explicitly, see #62. (And then we show the syntax to use so that the message isn't shown.)
TSchiefer commented 5 years ago
  1. I am not sure if that will work with such a function, since if we remove key columns before the flattening, the flattening can't take place, and afterwards, we don't know anymore which columns were key columns before (plus, we don't have a dm anymore). Or am I missing something?
  2. Currently I implemented cdm_flatten_to_tbl() in #66 in a way, that in case of an empty ellipsis, all FK-relations pointing away from start are being used to "haul the tables in". Would you not do this?
krlmlr commented 2 years ago

@moodymudskipper: I added this to the current milestone since we're working on dm_flatten_to_tbl() anyway in this milestone.

krlmlr commented 2 years ago

Let's split dm_flatten_to_tbl() and dm_squash_to_tbl() internally so that there is a function that creates a plan, and one that applies the plan. At some point we could discuss how to expose such plan objects (also used e.g. in wrapping/unwrapping) and how to compute on them.