raise error instead of warning when a user has missing data and add c…

[hawestra]

1. Raise user validation error instead of warning when a user's test data has missing data
2. Add this same validation check for a user's train data
3. Add tests

*To address this bug: Bug 2477311: [Live site bug]Add length validation for test / train datasets

Description

Checklist

• I have added screenshots above for all UI changes.
• I have added e2e tests for all UI changes.
• Documentation was updated if it was needed.

[codecov-commenter]

Codecov Report

Merging #2143 (9069723) into main (a7cb6ec) will increase coverage by 4.07%.
The diff coverage is 100.00%.

❗ Current head 9069723 differs from pull request most recent head 9b0c01c. Consider uploading reports for the commit 9b0c01c to get more accurate results

@@            Coverage Diff             @@
##             main    #2143      +/-   ##
==========================================
+ Coverage   88.14%   92.22%   +4.07%     
==========================================
  Files         129      103      -26     
  Lines        7270     5143    -2127     
==========================================
- Hits         6408     4743    -1665     
+ Misses        862      400     -462     

Flag	Coverage Δ
unittests	92.22% <100.00%> (+4.07%)	⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files	Coverage Δ
...sibleai/responsibleai/rai_insights/rai_insights.py	87.74% <100.00%> (+0.06%)	⬆️

... and 26 files with indirect coverage changes

[github-actions]

https://responsibleai.blob.core.windows.net/pullrequest/microsoft/responsible-ai-toolbox/hawestra/missingTrainDataValidation/dashboard/index.html

[romanlutz]

Not sure why this was ever a warning but this looks great, and thanks for adding tests 🙂👍🏻

[gaugup]

Why do we need the check on train data if it has missing values? We compute insights on test data only.

[hawestra]

Why do we need the check on train data if it has missing values? We compute insights on test data only.

looking at the error analysis manager constructor, we actually use the test data there :

but this highlights @imatiach-msft point, that if we fail because test data is missing but the user doesn't use error analysis, then we're failing their run for no reason. Right now, I kept the check for test data and train data to not have null values to keep the validation logic all in the same place but to Ilya's point, do you think we should even be failing and/or should this logic be in the respective manager code ?

[imatiach-msft]

my thoughts were just that we shouldn't fail at all if the user's pipeline can handle missing values already...

[github-actions]

https://responsibleai.blob.core.windows.net/pullrequest/microsoft/responsible-ai-toolbox/hawestra/missingTrainDataValidation/dashboard/index.html

[github-actions]

https://responsibleai.blob.core.windows.net/pullrequest/microsoft/responsible-ai-toolbox/hawestra/missingTrainDataValidation/dashboard/index.html

[github-actions]

https://responsibleai.blob.core.windows.net/pullrequest/microsoft/responsible-ai-toolbox/hawestra/missingTrainDataValidation/dashboard/index.html

[github-actions]

https://responsibleai.blob.core.windows.net/pullrequest/microsoft/responsible-ai-toolbox/hawestra/missingTrainDataValidation/dashboard/index.html

[github-actions]

https://responsibleai.blob.core.windows.net/pullrequest/microsoft/responsible-ai-toolbox/hawestra/missingTrainDataValidation/dashboard/index.html