-
Notifications
You must be signed in to change notification settings - Fork 5
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
Conversation
Code Coverage Summary
Results for commit: 458362d Minimum allowed coverage is ♻️ This comment has been updated with latest results |
Unit Tests Summary 1 files 12 suites 6s ⏱️ Results for commit 458362d. ♻️ This comment has been updated with latest results. |
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 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
@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. |
Closes #32
In particular please note the following design decisions of the initial implementation: