Skip to content

Conversation

@gogonzo
Copy link
Contributor

@gogonzo gogonzo commented Oct 30, 2025

Closes insightsengineering/NEST-roadmap#36

Check also with:

In this PR, due to the fact that data_extract_spec is not 1:1 convertible to picks, I propose to have an S3 method in each tm_ to switch between data_extract_spec/picks depending on the input type. Eventually, data_extract_spec variant will be removed completely after deprecation period.

@gogonzo gogonzo changed the title Redesign extraction@main picks Oct 30, 2025
@github-actions
Copy link
Contributor

github-actions bot commented Oct 30, 2025

Unit Tests Summary

  1 files   23 suites   16m 8s ⏱️
155 tests 149 ✅ 5 💤 1 ❌
524 runs  518 ✅ 5 💤 1 ❌

For more details on these failures, see this check.

Results for commit f8b884f.

♻️ This comment has been updated with latest results.

@github-actions
Copy link
Contributor

github-actions bot commented Oct 30, 2025

Unit Test Performance Difference

Test Suite $Status$ Time on main $±Time$ $±Tests$ $±Skipped$ $±Failures$ $±Errors$
shinytest2-tm_a_pca 💚 $157.36$ $-2.37$ $0$ $0$ $0$ $0$
shinytest2-tm_a_regression 💚 $70.59$ $-1.39$ $0$ $0$ $0$ $0$
shinytest2-tm_g_bivariate 💔 $90.72$ $+1.45$ $0$ $0$ $0$ $0$
shinytest2-tm_g_distribution 💚 $69.32$ $-6.92$ $0$ $0$ $0$ $0$
shinytest2-tm_misssing_data 💔 $57.17$ $+6.11$ $+5$ $0$ $0$ $-1$
shinytest2-tm_variable_browser 💔 $65.00$ $+19.41$ $0$ $0$ $-13$ $0$
Additional test case details
Test Suite $Status$ Time on main $±Time$ Test Case
examples 👶 $+0.01$ example_dot_color_palette_discrete.Rd
examples 👶 $+0.01$ example_dot_make_reactable_columns_call.Rd
examples 👶 $+0.01$ example_dot_plotly_selected_filter_children.Rd
examples 👶 $+0.01$ example_tm_t_reactables.Rd
examples 💀 $0.01$ $-0.01$ example_validate_input.Rd
shinytest2-tm_g_bivariate 💔 $36.31$ $+1.10$ e2e_tm_g_bivariate_Setting_encoding_inputs_produces_outputs_without_validation_errors.
shinytest2-tm_g_distribution 💚 $21.58$ $-5.48$ e2e_tm_g_distribution_Histogram_encoding_inputs_produce_output_without_validation_errors.
shinytest2-tm_g_distribution 💚 $12.86$ $-1.60$ e2e_tm_g_distribution_QQ_plot_encoding_inputs_produce_output_without_validation_errors.
shinytest2-tm_misssing_data 💔 $7.79$ $+6.04$ e2e_tm_missing_data_Validate_functionality_and_UI_response_for_By_Variable_Levels_
shinytest2-tm_variable_browser 💔 $9.57$ $+3.34$ e2e_tm_variable_browser_Selecting_treat_variable_as_factor_changes_the_table_headers.
shinytest2-tm_variable_browser 💔 $11.91$ $+2.56$ e2e_tm_variable_browser_changing_display_density_encoding_doesn_t_show_errors.
shinytest2-tm_variable_browser 💔 $13.26$ $+2.65$ e2e_tm_variable_browser_changing_outlier_definition_encoding_doesn_t_show_errors.
shinytest2-tm_variable_browser 💔 $12.21$ $+8.17$ e2e_tm_variable_browser_changing_plot_setting_encodings_doesn_t_show_errors.
shinytest2-tm_variable_browser 💔 $9.39$ $+1.32$ e2e_tm_variable_browser_content_is_displayed_correctly.
shinytest2-tm_variable_browser 💔 $8.67$ $+1.38$ e2e_tm_variable_browser_selection_of_categorical_variable_has_a_table_with_level_header.

Results for commit 9b0a1a2

♻️ This comment has been updated with latest results.

fix defaults
@llrs-roche
Copy link
Contributor

S3 methods can only dispatch according to one argument. This PR duplicates entire modules for a dispatch on one argument. Isn't it better to have a conversion or a warning on each argument? User might end up mixing the new and the old approach and that could throw off the dispatch mechanism.
Still this would require that some part of the module's code would need to change but I don't think it would be that much. Specially if the full release and removal of the old des object.

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.

Design data extract and data merge

4 participants