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

Fix unable to input decimal point in what if counterfactual feature text field #1459

Merged
merged 21 commits into from
Jun 13, 2022

Conversation

tongyu-microsoft
Copy link
Contributor

@tongyu-microsoft tongyu-microsoft commented Jun 1, 2022

Users are not able to input decimal point in what if counterfactual to update feature value. The root cause is, we check on column value on each update. One example is, when user wants to type in "18.5" and on typing "18.", toNumber() ignores the decimal point, result in user can only input "185" instead of the desired "18.5". The fix is to pass string to this.state.displayedData when the input field value ends with decimal point.

Description

Before:
Unable to input decimal point:
decimalCFAfter

After:
decimalCFAfter

Checklist

  • I have added screenshots above for all UI changes.
  • Documentation was updated if it was needed.
  • New tests were added or changes were manually verified.

romanlutz and others added 3 commits May 26, 2022 18:11
* fix error on machines with pyspark installed where passed dataframe is not spark pandas (#1415)

* add postbuild branch trigger (#1417)

* Fix causal UI strings according to classification/regression tasks (#1419)

* Fix causal UI strings according to classification/regression tasks

Signed-off-by: Gaurav Gupta <[email protected]>

* Fix lint error

Signed-off-by: Gaurav Gupta <[email protected]>

* Fix UI test

Signed-off-by: Gaurav Gupta <[email protected]>

* Fix description for model overview (#1425)

* fix description for model overview

* keep new description for new model overview

* fix failing to create error report when filter_features is empty list (#1421)

* filter out missing values from what if dropdown to prevent explanation dashboard from crashing (#1418)

* Remove |Set Value| blurb in case it is not availble in counterfactual panel (#1426)

Signed-off-by: Gaurav Gupta <[email protected]>

* Add y-axis description to counterfactual feature importance chart (#1423)

Signed-off-by: Gaurav Gupta <[email protected]>

Co-authored-by: xuke444 <[email protected]>

* Add the user class name to causal UI strings (#1422)

* Fix causal UI strings according to classification/regression tasks

Signed-off-by: Gaurav Gupta <[email protected]>

* Fix lint error

Signed-off-by: Gaurav Gupta <[email protected]>

* Fix UI test

Signed-off-by: Gaurav Gupta <[email protected]>

* Add the user class name to causal UI strings

Signed-off-by: Gaurav Gupta <[email protected]>

Co-authored-by: xuke444 <[email protected]>

* fix math.min / max for array size more than 10^7 (#1427)

Signed-off-by: Ke Xu <[email protected]>

* upgrade pytest and lightgbm to try to fix random pytest segfault test failures (#1424)

s

* fix flaky notebook causing build failures by adding retry logic (#1431)

* Upper bound SciKit-Learn to address freeze in causal (#1432)

<!--- Provide a general summary of your changes in the Title above -->

## Description

Replaces #1429  to address #1430 . Causal analysis is getting stuck with the latest release of SciKit-Learn. This contains:

- Test case which gets stuck with SciKit-Learn 1.1.0
- Upper bound on SciKit-Learn in `requirements.txt`

## Checklist

<!--- Make sure to satisfy all the criteria listed below. -->

- [x] I have added screenshots above for all UI changes.
- [x] Documentation was updated if it was needed.
- [x] New tests were added or changes were manually verified.


Signed-off-by: Richard Edgar <[email protected]>

* fix dependency chart axis updating with incorrect values in explanation dashboard (#1437)

* fix codecov and widget test screenshot uploads (#1428)

* release raiwidgets and responsibleai v0.18.2 (#1439)

* fix (#1441)

Signed-off-by: vinutha karanth <[email protected]>

* Fix cohort name conflict and not run few tests for AML (#1442)

* fix

Signed-off-by: vinutha karanth <[email protected]>

* lintfix

Signed-off-by: vinutha karanth <[email protected]>

* Few e2e tests changes to accommodate AML static tests (#1445)

* update

Signed-off-by: vinutha karanth <[email protected]>

* update

Signed-off-by: vinutha karanth <[email protected]>

* Fix locators logic for string features -  data explorer and model statistics components (#1446)

* update

Signed-off-by: vinutha karanth <[email protected]>

* update

Signed-off-by: vinutha karanth <[email protected]>

* fix

Signed-off-by: vinutha karanth <[email protected]>

* update

Signed-off-by: vinutha karanth <[email protected]>

* lintfix

Signed-off-by: vinutha karanth <[email protected]>

* fix

Signed-off-by: vinutha karanth <[email protected]>

* Add more unittests RAI dashboard input class  (#1448)

* Add unit tests for ResponsibleAIDashboardInput

Signed-off-by: Gaurav Gupta <[email protected]>

* Add more tests

Signed-off-by: Gaurav Gupta <[email protected]>

* Fix imports

Signed-off-by: Gaurav Gupta <[email protected]>

* Address code review comments

Signed-off-by: Gaurav Gupta <[email protected]>

* Update the way to get the length of elements obtained in e2e tests (#1450)

* update

Signed-off-by: vinutha karanth <[email protected]>

* update

Signed-off-by: vinutha karanth <[email protected]>

* skip what-if create tests for AML

Signed-off-by: vinutha karanth <[email protected]>

Co-authored-by: Ilya Matiach <[email protected]>
Co-authored-by: Gaurav Gupta <[email protected]>
Co-authored-by: xuke444 <[email protected]>
Co-authored-by: Richard Edgar <[email protected]>
Co-authored-by: Vinutha Karanth <[email protected]>
…ategorical')" (#1452)

* fix error on machines with pyspark installed where passed dataframe is not spark pandas (#1415)

* add postbuild branch trigger (#1417)

* Fix causal UI strings according to classification/regression tasks (#1419)

* Fix causal UI strings according to classification/regression tasks

Signed-off-by: Gaurav Gupta <[email protected]>

* Fix lint error

Signed-off-by: Gaurav Gupta <[email protected]>

* Fix UI test

Signed-off-by: Gaurav Gupta <[email protected]>

* Fix description for model overview (#1425)

* fix description for model overview

* keep new description for new model overview

* fix failing to create error report when filter_features is empty list (#1421)

* filter out missing values from what if dropdown to prevent explanation dashboard from crashing (#1418)

* Remove |Set Value| blurb in case it is not availble in counterfactual panel (#1426)

Signed-off-by: Gaurav Gupta <[email protected]>

* Add y-axis description to counterfactual feature importance chart (#1423)

Signed-off-by: Gaurav Gupta <[email protected]>

Co-authored-by: xuke444 <[email protected]>

* Add the user class name to causal UI strings (#1422)

* Fix causal UI strings according to classification/regression tasks

Signed-off-by: Gaurav Gupta <[email protected]>

* Fix lint error

Signed-off-by: Gaurav Gupta <[email protected]>

* Fix UI test

Signed-off-by: Gaurav Gupta <[email protected]>

* Add the user class name to causal UI strings

Signed-off-by: Gaurav Gupta <[email protected]>

Co-authored-by: xuke444 <[email protected]>

* fix math.min / max for array size more than 10^7 (#1427)

Signed-off-by: Ke Xu <[email protected]>

* upgrade pytest and lightgbm to try to fix random pytest segfault test failures (#1424)

s

* fix flaky notebook causing build failures by adding retry logic (#1431)

* Upper bound SciKit-Learn to address freeze in causal (#1432)

<!--- Provide a general summary of your changes in the Title above -->

## Description

Replaces #1429  to address #1430 . Causal analysis is getting stuck with the latest release of SciKit-Learn. This contains:

- Test case which gets stuck with SciKit-Learn 1.1.0
- Upper bound on SciKit-Learn in `requirements.txt`

## Checklist

<!--- Make sure to satisfy all the criteria listed below. -->

- [x] I have added screenshots above for all UI changes.
- [x] Documentation was updated if it was needed.
- [x] New tests were added or changes were manually verified.


Signed-off-by: Richard Edgar <[email protected]>

* fix dependency chart axis updating with incorrect values in explanation dashboard (#1437)

* fix codecov and widget test screenshot uploads (#1428)

* release raiwidgets and responsibleai v0.18.2 (#1439)

* fix (#1441)

Signed-off-by: vinutha karanth <[email protected]>

* Fix cohort name conflict and not run few tests for AML (#1442)

* fix

Signed-off-by: vinutha karanth <[email protected]>

* lintfix

Signed-off-by: vinutha karanth <[email protected]>

* Few e2e tests changes to accommodate AML static tests (#1445)

* update

Signed-off-by: vinutha karanth <[email protected]>

* update

Signed-off-by: vinutha karanth <[email protected]>

* Fix locators logic for string features -  data explorer and model statistics components (#1446)

* update

Signed-off-by: vinutha karanth <[email protected]>

* update

Signed-off-by: vinutha karanth <[email protected]>

* fix

Signed-off-by: vinutha karanth <[email protected]>

* update

Signed-off-by: vinutha karanth <[email protected]>

* lintfix

Signed-off-by: vinutha karanth <[email protected]>

* fix

Signed-off-by: vinutha karanth <[email protected]>

* fix

Signed-off-by: vinutha karanth <[email protected]>

* Add more unittests RAI dashboard input class  (#1448)

* Add unit tests for ResponsibleAIDashboardInput

Signed-off-by: Gaurav Gupta <[email protected]>

* Add more tests

Signed-off-by: Gaurav Gupta <[email protected]>

* Fix imports

Signed-off-by: Gaurav Gupta <[email protected]>

* Address code review comments

Signed-off-by: Gaurav Gupta <[email protected]>

* more update

Signed-off-by: vinutha karanth <[email protected]>

* Update the way to get the length of elements obtained in e2e tests (#1450)

* update

Signed-off-by: vinutha karanth <[email protected]>

* update

Signed-off-by: vinutha karanth <[email protected]>

* skip what-if create tests for AML

Signed-off-by: vinutha karanth <[email protected]>

Co-authored-by: Ilya Matiach <[email protected]>
Co-authored-by: Roman Lutz <[email protected]>
Co-authored-by: Gaurav Gupta <[email protected]>
Co-authored-by: xuke444 <[email protected]>
Co-authored-by: Richard Edgar <[email protected]>
@codecov-commenter
Copy link

codecov-commenter commented Jun 1, 2022

Codecov Report

Merging #1459 (2f29701) into main (93b9385) will not change coverage.
The diff coverage is n/a.

@@           Coverage Diff           @@
##             main    #1459   +/-   ##
=======================================
  Coverage   87.57%   87.57%           
=======================================
  Files         110      110           
  Lines        5093     5093           
=======================================
  Hits         4460     4460           
  Misses        633      633           
Flag Coverage Δ
unittests 87.57% <ø> (ø)

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


Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 93b9385...2f29701. Read the comment docs.

3 similar comments
@gaugup
Copy link
Contributor

gaugup commented Jun 2, 2022

Can you please capture this change in the e2e tests?

…el (#1458)

* fix error on machines with pyspark installed where passed dataframe is not spark pandas (#1415)

* add postbuild branch trigger (#1417)

* Fix causal UI strings according to classification/regression tasks (#1419)

* Fix causal UI strings according to classification/regression tasks

Signed-off-by: Gaurav Gupta <[email protected]>

* Fix lint error

Signed-off-by: Gaurav Gupta <[email protected]>

* Fix UI test

Signed-off-by: Gaurav Gupta <[email protected]>

* Fix description for model overview (#1425)

* fix description for model overview

* keep new description for new model overview

* fix failing to create error report when filter_features is empty list (#1421)

* filter out missing values from what if dropdown to prevent explanation dashboard from crashing (#1418)

* Remove |Set Value| blurb in case it is not availble in counterfactual panel (#1426)

Signed-off-by: Gaurav Gupta <[email protected]>

* Add y-axis description to counterfactual feature importance chart (#1423)

Signed-off-by: Gaurav Gupta <[email protected]>

Co-authored-by: xuke444 <[email protected]>

* Add the user class name to causal UI strings (#1422)

* Fix causal UI strings according to classification/regression tasks

Signed-off-by: Gaurav Gupta <[email protected]>

* Fix lint error

Signed-off-by: Gaurav Gupta <[email protected]>

* Fix UI test

Signed-off-by: Gaurav Gupta <[email protected]>

* Add the user class name to causal UI strings

Signed-off-by: Gaurav Gupta <[email protected]>

Co-authored-by: xuke444 <[email protected]>

* fix math.min / max for array size more than 10^7 (#1427)

Signed-off-by: Ke Xu <[email protected]>

* upgrade pytest and lightgbm to try to fix random pytest segfault test failures (#1424)

s

* fix flaky notebook causing build failures by adding retry logic (#1431)

* Upper bound SciKit-Learn to address freeze in causal (#1432)

<!--- Provide a general summary of your changes in the Title above -->

## Description

Replaces #1429  to address #1430 . Causal analysis is getting stuck with the latest release of SciKit-Learn. This contains:

- Test case which gets stuck with SciKit-Learn 1.1.0
- Upper bound on SciKit-Learn in `requirements.txt`

## Checklist

<!--- Make sure to satisfy all the criteria listed below. -->

- [x] I have added screenshots above for all UI changes.
- [x] Documentation was updated if it was needed.
- [x] New tests were added or changes were manually verified.


Signed-off-by: Richard Edgar <[email protected]>

* fix dependency chart axis updating with incorrect values in explanation dashboard (#1437)

* fix codecov and widget test screenshot uploads (#1428)

* release raiwidgets and responsibleai v0.18.2 (#1439)

* fix (#1441)

Signed-off-by: vinutha karanth <[email protected]>

* Fix cohort name conflict and not run few tests for AML (#1442)

* fix

Signed-off-by: vinutha karanth <[email protected]>

* lintfix

Signed-off-by: vinutha karanth <[email protected]>

* Few e2e tests changes to accommodate AML static tests (#1445)

* update

Signed-off-by: vinutha karanth <[email protected]>

* update

Signed-off-by: vinutha karanth <[email protected]>

* Fix locators logic for string features -  data explorer and model statistics components (#1446)

* update

Signed-off-by: vinutha karanth <[email protected]>

* update

Signed-off-by: vinutha karanth <[email protected]>

* fix

Signed-off-by: vinutha karanth <[email protected]>

* update

Signed-off-by: vinutha karanth <[email protected]>

* lintfix

Signed-off-by: vinutha karanth <[email protected]>

* fix

Signed-off-by: vinutha karanth <[email protected]>

* Add more unittests RAI dashboard input class  (#1448)

* Add unit tests for ResponsibleAIDashboardInput

Signed-off-by: Gaurav Gupta <[email protected]>

* Add more tests

Signed-off-by: Gaurav Gupta <[email protected]>

* Fix imports

Signed-off-by: Gaurav Gupta <[email protected]>

* Address code review comments

Signed-off-by: Gaurav Gupta <[email protected]>

* Update the way to get the length of elements obtained in e2e tests (#1450)

* update

Signed-off-by: vinutha karanth <[email protected]>

* update

Signed-off-by: vinutha karanth <[email protected]>

* skip what-if create tests for AML

Signed-off-by: vinutha karanth <[email protected]>

* update cf predicted class to non-editable text

Co-authored-by: Ilya Matiach <[email protected]>
Co-authored-by: Roman Lutz <[email protected]>
Co-authored-by: Gaurav Gupta <[email protected]>
Co-authored-by: xuke444 <[email protected]>
Co-authored-by: Richard Edgar <[email protected]>
Co-authored-by: Vinutha Karanth <[email protected]>
@tongyu-microsoft
Copy link
Contributor Author

Add E2E test for targetClass in describeWhatIfCreate.ts

1 similar comment
@tongyu-microsoft tongyu-microsoft changed the base branch from postbuild to main June 6, 2022 18:57
@tongyu-microsoft tongyu-microsoft changed the base branch from main to postbuild June 6, 2022 18:58
@tongyu-microsoft tongyu-microsoft changed the base branch from postbuild to main June 6, 2022 23:23
@tongyu-microsoft
Copy link
Contributor Author

Can you please capture this change in the e2e tests?

Tests are added

@tongyu-microsoft
Copy link
Contributor Author

Add E2E test for targetClass in describeWhatIfCreate.ts

Tests are added

1 similar comment
2 similar comments
@tongyu-microsoft tongyu-microsoft merged commit 9342c46 into main Jun 13, 2022
@tongyu-microsoft tongyu-microsoft deleted the tongy/fractionErrorInCF branch June 13, 2022 17:09
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.

6 participants