Skip to content

fix label logic #895

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.

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

Merged
merged 2 commits into from
Jul 15, 2025
Merged

fix label logic #895

merged 2 commits into from
Jul 15, 2025

Conversation

donyunardi
Copy link
Contributor

Fixes #510

  • Improve the logic to define params_names
Example Code 1
data <- teal_data()
data <- within(data, {
  iris <- iris
})
#'
app <- init(
  data = data,
  modules = list(
    tm_g_distribution(
      dist_var = data_extract_spec(
        dataname = "iris",
        select = select_spec(variable_choices("iris"), "Petal.Length")
      )
    )
  )
)
if (interactive()) {
  shinyApp(app$ui, app$server)
}
Example Code 2 where nothing is selected
data <- teal_data()
data <- within(data, {
  ADSL <- teal.data::rADSL
})
join_keys(data) <- default_cdisc_join_keys[names(data)]
#'
vars1 <- choices_selected(
  variable_choices(data[["ADSL"]], c("ARM", "COUNTRY", "SEX")),
  selected = NULL
)
#'
app <- init(
  data = data,
  modules = modules(
    tm_g_distribution(
      dist_var = data_extract_spec(
        dataname = "ADSL",
        select = select_spec(
          choices = variable_choices(data[["ADSL"]], c("AGE", "BMRKR1")),
          selected = NULL,
          multiple = FALSE,
          fixed = FALSE
        )
      ),
      strata_var = data_extract_spec(
        dataname = "ADSL",
        filter = filter_spec(
          vars = vars1,
          multiple = TRUE
        )
      ),
      group_var = data_extract_spec(
        dataname = "ADSL",
        filter = filter_spec(
          vars = vars1,
          multiple = TRUE
        )
      )
    )
  )
)
if (interactive()) {
  shinyApp(app$ui, app$server)
}

Copy link
Contributor

github-actions bot commented Jul 14, 2025

Unit Tests Summary

  1 files  22 suites   2s ⏱️
144 tests 29 ✅ 115 💤 0 ❌
182 runs  67 ✅ 115 💤 0 ❌

Results for commit edeee4e.

♻️ This comment has been updated with latest results.

Copy link
Contributor

badge

Code Coverage Summary

Filename                      Stmts    Miss  Cover    Missing
--------------------------  -------  ------  -------  ---------------------------------------
R/tm_a_pca.R                    889     889  0.00%    139-1161
R/tm_a_regression.R             773     773  0.00%    178-1056
R/tm_data_table.R               201     201  0.00%    100-349
R/tm_file_viewer.R              173     173  0.00%    47-255
R/tm_front_page.R               144     133  7.64%    77-247
R/tm_g_association.R            344     344  0.00%    159-578
R/tm_g_bivariate.R              698     434  37.82%   331-826, 867, 978, 995, 1013, 1024-1046
R/tm_g_distribution.R          1127    1127  0.00%    156-1431
R/tm_g_response.R               369     369  0.00%    177-625
R/tm_g_scatterplot.R            734     734  0.00%    260-1098
R/tm_g_scatterplotmatrix.R      297     278  6.40%    198-532, 593, 607
R/tm_missing_data.R            1119    1119  0.00%    124-1420
R/tm_outliers.R                1061    1061  0.00%    163-1379
R/tm_t_crosstable.R             285     285  0.00%    175-509
R/tm_variable_browser.R         803     798  0.62%    89-1044, 1082-1265
R/utils.R                       151     135  10.60%   87-272, 302-338, 350-359, 364, 378-397
R/zzz.R                           2       2  0.00%    2-3
TOTAL                          9170    8855  3.44%

Diff against main

Filename                 Stmts    Miss  Cover
---------------------  -------  ------  --------
R/tm_g_distribution.R       +4      +4  +100.00%
TOTAL                       +4      +4  -0.00%

Results for commit: edeee4e

Minimum allowed coverage is 80%

♻️ This comment has been updated with latest results

Copy link
Contributor

@llrs-roche llrs-roche left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The changes make possible to open the app now with the devel version of all packages.

On the second app I see some UI overlap:

image

Note that there is a change on the manuals that is unrelated. Remove it and we can merge it.

@llrs-roche llrs-roche self-assigned this Jul 15, 2025
@averissimo averissimo assigned averissimo and unassigned averissimo Jul 15, 2025
@donyunardi
Copy link
Contributor Author

Note that there is a change on the manuals that is unrelated. Remove it and we can merge it.

As discussed, the manual update was done by CI/CD. It could be something that was missed initially.

On the second app I see some UI overlap:

I'll open a separate issue for this.

@donyunardi donyunardi mentioned this pull request Jul 15, 2025
@donyunardi donyunardi merged commit c0428b9 into main Jul 15, 2025
28 of 29 checks passed
@donyunardi donyunardi deleted the 510_fix_label@main branch July 15, 2025 21:25
@github-actions github-actions bot locked and limited conversation to collaborators Jul 15, 2025
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

labels in distribution module resetting
3 participants