Skip to content

fix: read mapping, filters, ratio_table, and units from YAML in get_settings_code()#1226

Open
Gero1999 wants to merge 3 commits intomainfrom
1189-bug/get-settings-code-read-yaml-fields
Open

fix: read mapping, filters, ratio_table, and units from YAML in get_settings_code()#1226
Gero1999 wants to merge 3 commits intomainfrom
1189-bug/get-settings-code-read-yaml-fields

Conversation

@Gero1999
Copy link
Copy Markdown
Collaborator

Issue

Closes #1189

Description

get_settings_code() used a hardcoded default_mapping parameter and ignored filters, ratio_table, units_table, and extra_vars_to_keep stored in the settings YAML. Generated R scripts would use wrong mapping and skip filters, ratios, and unit overrides.

Changes:

  • Read mapping, filters, ratio_table, units_table from the YAML when present
  • Derive extra_vars_to_keep from mapping$select_Grouping_Variables (consistent with the UI path in nca_results.R)
  • Read time_duplicate_keys from the YAML instead of hardcoding NULL
  • Rename default_mapping to .default_mapping (internal, not exported) and use it as fallback for legacy YAML files
  • Remove the mapping parameter from the public API

Definition of Done

  • get_settings_code() reads mapping from YAML, falls back to .default_mapping for legacy files
  • get_settings_code() includes applied_filters from YAML in the session object
  • get_settings_code() includes ratio_table from YAML in the session object
  • get_settings_code() includes units_table from YAML in the session object
  • get_settings_code() derives extra_vars_to_keep from mapping
  • Generated R scripts correctly apply mapping, filters, units, and ratio calculations

How to test

  1. Generate a settings YAML from the app (with custom mapping, filters, ratio table, and units)
  2. Call get_settings_code(settings_file_path, data_path) — no mapping argument needed
  3. Inspect the generated R script: it should contain the custom mapping values, filters, ratio table, and units from the YAML
  4. Test with a legacy YAML (no mapping key) — should fall back to CDISC defaults

Contributor checklist

  • Code passes lintr checks
  • Code passes all unit tests
  • New logic covered by unit tests
  • New logic is documented
  • App or package changes are reflected in NEWS
  • Package version is incremented
  • R script works with the new implementation (if applicable)
  • Settings upload works with the new implementation (if applicable)
  • If any .scss change was done, run data-raw/compile_css.R

Notes to reviewer

Gero1999 and others added 3 commits April 15, 2026 12:30
… in get_settings_code()

get_settings_code() used a hardcoded default_mapping parameter and
ignored filters, ratio_table, units_table, and extra_vars_to_keep
stored in the settings YAML. The generated R scripts would use wrong
mapping and skip filters, ratios, and unit overrides.

Now all fields are read from the YAML when present, with
.default_mapping as fallback for legacy files. extra_vars_to_keep is
derived from the mapping's Grouping_Variables. The mapping parameter
has been removed from the public API.

Closes #1189

Co-authored-by: Ona <no-reply@ona.com>
@Gero1999 Gero1999 marked this pull request as ready for review April 15, 2026 14:33
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Bug: get_settings_code() doesn't read mapping, filters, or ratio_table from settings YAML

1 participant