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

[300 line limit] Refactor CounterfactualChart #1624

Merged
merged 25 commits into from
Aug 11, 2022

Conversation

tongyu-microsoft
Copy link
Contributor

@tongyu-microsoft tongyu-microsoft commented Aug 9, 2022

This PR refactors CounterfactualChart, so line of code for CounterfactualChart is reduced from 938 lines to 249 lines.

Description

For CounterfactualList.tsx, this PR creates a few new components as children of CounterfactualList component. So each file now has less than 300 lines of code.

This PR basically does the following:
1.Every new CounterfactualXXX components in the PR is originally from CounterfactualChart
2. Rename CounterfactualChart to CounterfactualComponent, as it does include CounterfactualChart, LocalImportanceChart and error dialog etc -> so CounterfactualComponent is the parent component in my PR.
3. CounterfactualComponent is parent of CounterfactualChartWithLegend, CounterfactualLocalImportanceChart and CounterfactualErrorDialog
4. CounterfactualChartWithLegend is parent of CounterfactualChart and CounterfactualChartLegend
5. Moved all getXXX functions which doesn't contain any setState in the function, to src/util folder.

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.

@tongyu-microsoft tongyu-microsoft changed the title Tongy/limit300lines part4 [300 line limit] Refactor CounterfactualChart Aug 9, 2022
@codecov-commenter
Copy link

codecov-commenter commented Aug 10, 2022

Codecov Report

Merging #1624 (d569a35) into main (6311e46) will not change coverage.
The diff coverage is n/a.

@@           Coverage Diff           @@
##             main    #1624   +/-   ##
=======================================
  Coverage   88.86%   88.86%           
=======================================
  Files         105      105           
  Lines        5449     5449           
=======================================
  Hits         4842     4842           
  Misses        607      607           
Flag Coverage Δ
unittests 88.86% <ø> (ø)

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

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

1 similar comment
@tongyu-microsoft tongyu-microsoft merged commit 66fd5b7 into main Aug 11, 2022
@tongyu-microsoft tongyu-microsoft deleted the tongy/limit300linesPart4 branch August 11, 2022 05:28
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.

4 participants