Skip to content

Conversation

averissimo
Copy link
Contributor

Pull Request

Fixes #561

Changes description

Prefer using other parameter of the function instead of hardcoded value.

note: The second call of rule_diff() is redundant and could be removed, I haven't as it could have been added to prevent some edge case.

@averissimo averissimo changed the title fix: remove hardcoded on validation to correct #561 561 Allow the selection of a categorical variable in tm_outliers Aug 8, 2023
@averissimo averissimo added core bug Something isn't working labels Aug 8, 2023
@github-actions
Copy link
Contributor

github-actions bot commented Aug 8, 2023

Unit Tests Summary

  1 files    5 suites   0s ⏱️
16 tests 16 ✔️ 0 💤 0
49 runs  49 ✔️ 0 💤 0

Results for commit 6e6ce0f.

♻️ This comment has been updated with latest results.

@github-actions
Copy link
Contributor

github-actions bot commented Aug 8, 2023

badge

Code Coverage Summary

Filename                      Stmts    Miss  Cover    Missing
--------------------------  -------  ------  -------  ----------------------------------------------
R/tm_a_pca.R                    828     828  0.00%    72-1024
R/tm_a_regression.R             714     714  0.00%    92-889
R/tm_data_table.R               171     171  0.00%    58-281
R/tm_file_viewer.R              172     172  0.00%    39-241
R/tm_front_page.R               127     116  8.66%    63-205
R/tm_g_association.R            331     331  0.00%    89-478
R/tm_g_bivariate.R              652     492  24.54%   124-702, 750, 756, 760, 871, 888, 906, 926-948
R/tm_g_distribution.R          1025    1025  0.00%    110-1257
R/tm_g_response.R               347     347  0.00%    85-496
R/tm_g_scatterplot.R            716     716  0.00%    160-967
R/tm_g_scatterplotmatrix.R      278     259  6.83%    78-373, 427, 441
R/tm_missing_data.R            1034    1034  0.00%    58-1228
R/tm_outliers.R                 942     942  0.00%    76-1140
R/tm_t_crosstable.R             252     252  0.00%    81-371
R/tm_variable_browser.R         820     815  0.61%    61-1309
R/utils.R                       122     122  0.00%    74-351
R/zzz.R                           1       1  0.00%    2
TOTAL                          8532    8337  2.29%

Diff against main

Filename           Stmts    Miss  Cover
---------------  -------  ------  --------
R/tm_outliers.R       -2      -2  +100.00%
TOTAL                 -2      -2  +0.00%

Results for commit: c6a719a

Minimum allowed coverage is 80%

♻️ This comment has been updated with latest results

@chlebowa
Copy link
Contributor

chlebowa commented Aug 8, 2023

The fix is good 👍

Which part of the rule function do you think is redundant?

@averissimo
Copy link
Contributor Author

@chlebowa never mind about the redundancy, it totally makes sense to show the error in both boxes, in particular, if the message doesn't appear in duplicate.

Thanks for the ?teal::validate_inputs tip!!

@averissimo averissimo enabled auto-merge (squash) August 8, 2023 14:54
Copy link
Contributor

@chlebowa chlebowa left a comment

Choose a reason for hiding this comment

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

👌

@averissimo averissimo merged commit 32eaa7c into main Aug 8, 2023
@averissimo averissimo deleted the fix_outlier_validation branch August 8, 2023 15:03
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working core

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Bug]: tm_outliers categorical variable selection always results in error

2 participants