Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Naming pattern from .names ignored in dm_from_con() #1933

Closed
owenjonesuob opened this issue Jul 24, 2023 · 1 comment · May be fixed by #1934
Closed

Naming pattern from .names ignored in dm_from_con() #1933

owenjonesuob opened this issue Jul 24, 2023 · 1 comment · May be fixed by #1934
Milestone
learn

Comments

@owenjonesuob
Copy link
Contributor

A new .names parameter was added to dm_from_con() in 1.0.6 (#1790).

This is used in dm:::get_src_tbl_names() to construct names to be used in the data model.

However, in dm_from_con(), we then completely ignore those names and use the table name instead:

  #! We work hard to set names here...
  if (is_null(table_names)) {
    src_tbl_names <- get_src_tbl_names(src, ..., names = .names)
  } else {
    src_tbl_names <- table_names
  }

  #! ... and then override them anyway!
  nms <- purrr::map_chr(src_tbl_names, ~ .x@name[["table"]])

  tbls <-
    set_names(src_tbl_names, nms) %>%
    quote_ids(con) %>%
    map(possibly(tbl, NULL), src = src)
@owenjonesuob owenjonesuob mentioned this issue Jul 24, 2023
Draft
@krlmlr krlmlr added this to the learn milestone Aug 20, 2023
@owenjonesuob
Copy link
Contributor Author

I think this may have been fixed a little while ago by #2148 😇

I'll leave the linked (draft) PR open, because there is still an open question there about how .names and table_names ought to be specified simultaneously!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
None yet
Milestone
learn
Development

Successfully merging a pull request may close this issue.

fix: Always use get_src_tbl_names() to fetch tbl Ids within dm_from_con() owenjonesuob/dm
2 participants
@krlmlr @owenjonesuob