Skip to content
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

Column Order Checking #110

Merged
merged 13 commits into from
Jul 26, 2024
Merged

Column Order Checking #110

merged 13 commits into from
Jul 26, 2024

Conversation

gowerc
Copy link
Owner

@gowerc gowerc commented Jul 17, 2024

Closes #32

In particular please note the following design decisions of the initial implementation:

  • Takes the order of the columns prior to removing any for any reason (e.g. mode differences)
  • Removes columns that don't exist in both datasets from the printed output however the column order is derived before the columns are removed
  • The test is enabled by default with an additional option to disable it

Copy link
Contributor

github-actions bot commented Jul 17, 2024

badge

Code Coverage Summary

Filename                Stmts    Miss  Cover    Missing
--------------------  -------  ------  -------  --------------------------
R/ascii_tables.R          105       8  92.38%   10, 148, 158, 163-166, 211
R/cast_variables.R         49       0  100.00%
R/diffdf.R                188      18  90.43%   358-375, 402
R/generate_keyname.R       10       1  90.00%   16
R/identify.R              152       8  94.74%   283-290
R/is_different.R           52       0  100.00%
R/issuerows.R              40       0  100.00%
R/issues.R                 17       1  94.12%   51
R/misc_functions.R         34       2  94.12%   9, 13
R/print.R                  20       0  100.00%
TOTAL                     667      38  94.30%

Results for commit: 458362d

Minimum allowed coverage is 80%

♻️ This comment has been updated with latest results

Copy link
Contributor

github-actions bot commented Jul 17, 2024

Unit Tests Summary

  1 files   12 suites   6s ⏱️
 49 tests  48 ✅ 1 💤 0 ❌
573 runs  566 ✅ 7 💤 0 ❌

Results for commit 458362d.

♻️ This comment has been updated with latest results.

Copy link
Collaborator

@kieranjmartin kieranjmartin left a comment

Choose a reason for hiding this comment

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

I think you should include testing of the full diffdf. I did test it myself quickly, but if you include columns that aren't in both, and also if you include a key, this could theoretically break this. Right now it seems to handle it, just thinking about future proofing this functionality

R/diffdf.R Outdated Show resolved Hide resolved
tests/testthat/test-identify_column_order.R Show resolved Hide resolved
@gowerc
Copy link
Owner Author

gowerc commented Jul 25, 2024

@kieranjmartin - I think I've addressed all your comments now. Only thing I haven't been able to do is setup a test for out of order columns if one of they keys is missing due to #113 . That being said I do have a full test where keys are specified and feel comfortable that this is working as expected.

@gowerc gowerc merged commit 933f6dc into master Jul 26, 2024
23 checks passed
@gowerc gowerc deleted the 32-check-column-order branch July 26, 2024 11:35
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.

Show differences in column order between dataframes
2 participants