1198: Add variable_max_size to variables metadata for enhanced data insights#1679
Conversation
alexfurmenkov
changed the title
There was a problem hiding this comment.
Could you please add testing for this new meta variable covering edge cases?
| # Check if the rule requires variable_max_size | ||
| if ( | ||
| self.rule | ||
| and self.rule.get("output_variables") |
There was a problem hiding this comment.
i think we want to change the conditions for this--a rule using variable_max_size may not put it in the output variables and cause issue downstream.
We would want to iterate through conditions, looking for variable_max_size--this shouldnt be too costly since most rules dont have too many conditions. We will not know the shape of conditions though--could be any/all, nested so we will need recursion
There was a problem hiding this comment.
@SFJohnson24 Could you check the new logic?
RamilCDISC
left a comment
RamilCDISC
left a comment
There was a problem hiding this comment.
Thanks for the tests. All seems fine to me now, but i think we will need to update the documentation too. Could you also please attach any rule and dataset (if you have) you used while developing this ticket.
|
Hi, It's the rule and the negative dataset. |
|
I have completed my review. But before approval we need to update the documentation to add this new variable. Once updated I will approve and final comment. |
RamilCDISC
left a comment
RamilCDISC
left a comment
There was a problem hiding this comment.
The PR adds variable_max_size to meta variables so a comparision for column size and actual max size of the data in the column can be done. The valdiaitonw as done by:
- Reviewing the PR for any unwanted changed or comments.
- Reviewing the code in accordance with AC.
- Ensuring proper testing is implemented.
- Ensuring related documentation is updated.
- Ensuring all unit and regression testing pass.
- Running manual validation using positive dataset.
- Running manual validation using negative dataset.
- Covering edge cases such as:
- missing value in a column
- missing value of dataset size
- all columns across datasets empty
- checking functionality on char column
- checking functionality on num column
- checking functionality on --Column Names across different datasets
- checking functionality on fixed columns like USUBJID which are same across the datasets.
3 participants
No description provided.