-
-
Notifications
You must be signed in to change notification settings - Fork 15
Add tables to show-r-code
in distribution, scatterplot and outliers modules
#574
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
Conversation
Code Coverage Summary
Diff against main
Results for commit: 72ba9c9 Minimum allowed coverage is ♻️ This comment has been updated with latest results |
Hi @averissimo, just wanted to let you know that you can find useful tips on naming conventions for stage dependencies at this link: GitHub Naming Conventions. When you use a branch name like "print_tables_show_r_code@main," the staged.dependencies tool will check branches in other repositories. |
R/tm_outliers.R
Outdated
) | ||
) %>% | ||
# used to display table when running show-r-code code | ||
teal.code::eval_code(quote(ANL_OUTLIER_EXTENDED[ANL_OUTLIER_EXTENDED$is_outlier_selected, ])) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ANL_OUTLIER_EXTENDED[ANL_OUTLIER_EXTENDED$is_outlier_selected, ]
Show all columns in the dataset, including those not displayed on the dashboard.
I believe that the code should print a table that displays on the dashboard, as well as any user-selected columns.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@kartikeyakirar this may have a UX side effect as any change to the columns being selected will clear the selected data points.
My suggestion is to add the outlier table print after the plot call, otherwise, any column change will trigger reactivity in the <plot>_q
and re-render the plot.
The downside is that this approach requires duplicate code only (column selection)
WDYT?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Show all columns in the dataset, including those not displayed on the dashboard.
this problem is fixed. I can see table displayed on dashboard.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
any change to the columns being selected will clear the selected data points.
I believe there's no necessity for us to proceed with this step, as we're unable print user selections within the ggplot. Thus, there's no need to gather the selected datapoints in reproducible code. In my opinion, doing so might lead to confusion when working with reproducible code. I'd appreciate your input on this, @cicdguy, particularly with regard to displaying datapoints from outlier selections in the plot.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'll defer to @donyunardi or @gogonzo to voice their opinions on this. I don't think I'm qualified to speak to this 👨🏽🎓
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
any change to the columns being selected will clear the selected data points.
My statement wasn't super clear. This relates to the placement of the table-print
code in qenv
, as if we place the code for table-print
before the plot-print
, then it will add reactivity that re-renders the plot every time a column is selected/deselected (which causes all brushed selection being lost)
Hence, the table-print
is located after the plot-print
.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hence, the table-print is located after the plot-print.
The solution is looking good! Just wanted to mention that the placement of the table code seems alright in my opinion.
20d4472
to
43ad79b
Compare
@averissimo can you make similar change to your PR to Pass GitHub checks. d711d17 |
d540cf1
to
77be5b3
Compare
Hey @averissimo, I still have some concerns about this point. Could we add another task for discussion regarding this matter? |
Pull Request
Fixes #558
Changes description
Adds tables to
Show R code
shown on screen for different modulestm_g_distribution
: simple "print" of existing table to qenvtm_g_scatterplot.R
: simple "print" of existing table to qenvtm_outliers.R
How to test
exploratory
application ofteal.gallery
Show R code
Outliers
table won't deselect the data points selection