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

RAIInsights: prevent failures on optional methods & fix feature range formatting for timestamps #2258

Merged
merged 8 commits into from
Aug 19, 2023

Conversation

romanlutz
Copy link
Contributor

Description

With optional methods such as forecast_quantiles we need to be able to check if the model has the method at all without getting a failure. This PR adjusts the handling slightly to prevent unintended issues.

Secondly, the feature ranges need to be specified in a JSON-serializable format, so we're storing them as strings now.

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
Copy link

codecov-commenter commented Aug 18, 2023

Codecov Report

Merging #2258 (b7f5af3) into main (51a2982) will increase coverage by 0.84%.
The diff coverage is 80.00%.

@@            Coverage Diff             @@
##             main    #2258      +/-   ##
==========================================
+ Coverage   92.26%   93.10%   +0.84%     
==========================================
  Files         103      103              
  Lines        5171     5179       +8     
==========================================
+ Hits         4771     4822      +51     
+ Misses        400      357      -43     
Flag Coverage Δ
unittests 93.10% <80.00%> (+0.84%) ⬆️

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

Files Changed Coverage Δ
...sibleai/responsibleai/rai_insights/rai_insights.py 90.36% <80.00%> (+2.62%) ⬆️

... and 1 file with indirect coverage changes

Copy link
Contributor

@gaugup gaugup left a comment

Choose a reason for hiding this comment

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

Could add unit tests for these changes?

1 similar comment
@romanlutz
Copy link
Contributor Author

Could add unit tests for these changes?

Added tests as requested.

@romanlutz romanlutz requested a review from gaugup August 19, 2023 00:13
@romanlutz romanlutz merged commit f07d675 into main Aug 19, 2023
@romanlutz romanlutz deleted the romanlutz/rai_insights_fix branch August 19, 2023 00:14
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.

5 participants