fix: pass naming pattern through to dm_learn_from_db() (#2213)#2214
fix: pass naming pattern through to dm_learn_from_db() (#2213)#2214owenjonesuob wants to merge 6 commits intocynkra:mainfrom
Conversation
Current Aviator status
This pull request is currently open (not queued). How to mergeTo merge this PR, comment
See the real-time status of this PR on the
Aviator webapp.
Use the Aviator Chrome Extension
to see the status of your PR within GitHub.
|
owenjonesuob
force-pushed
the
b-pass-names-pattern-to-learn
branch
2 times, most recently
from
14afa3c to
192226a
Compare
krlmlr
force-pushed
the
b-pass-names-pattern-to-learn
branch
from
192226a to
a1119ed
Compare
There was a problem hiding this comment.
Pull Request Overview
This PR fixes an issue where the naming pattern was not being passed through to dm_learn_from_db(). The fix involves refactoring the "smart default" logic for naming patterns from get_src_tbl_names() to dm_from_con() and ensuring consistent use of names_pattern as the parameter name throughout the codebase.
Key changes:
- Moved smart default logic for naming patterns from
get_src_tbl_names()todm_from_con() - Standardized parameter name from
namestonames_patternacross functions - Updated
dm_learn_from_db()to use the correct column names with dot prefixes for glue templating
Reviewed Changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| R/learn.R | Updated dm_learn_from_db() parameter name and column selection to use dot-prefixed names |
| R/dm_from_con.R | Added smart default logic for naming patterns and updated function calls to use names_pattern |
| R/db-helpers.R | Removed smart default logic and updated parameter name to names_pattern |
| tests/testthat/test-db-helpers.R | Updated test calls to use names_pattern parameter instead of names |
| #' } | ||
| #' @autoglobal | ||
| dm_learn_from_db <- function(dest, dbname = NA, schema = NULL, name_format = "{table}") { | ||
| dm_learn_from_db <- function(dest, dbname = NA, schema = NULL, names_pattern = "{.table}") { |
|
@claude: Fix tests. |
|
@copilot: Fix tests. |
|
@owenjonesuob: Thank you for your patience. |
|
Claude encountered an error —— View job I'll analyze this and get back to you. |
2 participants
Fixes #2213.
I had to move the "smart default" logic for
.namesup one level, fromget_src_tbl_names()intodm_from_con()- but I think that is a better place to keep it anyway, since that's where its behaviour is documented.I've also made sure
names_patternis used consistently as the "internal" name for this argument, wherever we see this "naming pattern" concept.