-
Notifications
You must be signed in to change notification settings - Fork 27
1345: correctly preprocess supp and relrec datasets #1542
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
1345: correctly preprocess supp and relrec datasets #1542
Conversation
…d-relrec-datasets
SFJohnson24
left a comment
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.
this looks good--can you make 1 test in tests/unit/test_dataset_preprocessor for the SUPPDM logic? I appreciate the regression tests but want this also in the unittests
…nd-relrec-datasets' into 1345-correctly-preprocess-supp-and-relrec-datasets
SFJohnson24
left a comment
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.
PR adds improved suppDM relrec handling. Behaves as expected, adds regression test and unit tests
RamilCDISC
left a comment
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.
The PR fixes the preprocessing for supp and relrec datasets. The validation was done by:
- Reviewing the PR for any unwanted code or comments.
- Reviewing the PR logic in accordance with AC.
- Ensuring all unit and regression testing pass.
- Ensuring new testing cover required test cases.
- manual validation using rule editor with positive and negative dataset.
- Manual validation using CLI with postive and negative dataset.
- Running and ensuring the new regression test.
No description provided.