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] Update array_math.py #2398

Merged
merged 3 commits into from Mar 15, 2023

Conversation

mglowacki100
Copy link
Contributor

Provide support for models that return pandas series for predict, predict_proba (e.g. autogluon) by fixing sequence_to_numpy function from utils/array_math.py

Reference Issues/PRs

Partially Fixes #2364 as discussed with @noamzbr

What does this implement/fix? Explain your changes.

Fixing is just by adding additional case for pd.Series. Pandas series has method to to_numpy after this just flatten.

I didn't find specific test for sequence_to_numpy, so I've locally tested it end-to-end with autogluon:
https://colab.research.google.com/gist/mglowacki100/49ba6dca46f75ab494068d5b26a6a39c/autogluon_with_deepchecks_test.ipynb

Provide support for models that return pandas series for predict, predict_proba
@mglowacki100 mglowacki100 requested a review from a team as a code owner March 14, 2023 14:09
@CLAassistant
Copy link

CLAassistant commented Mar 14, 2023

CLA assistant check
All committers have signed the CLA.

@noamzbr noamzbr added the feature Feature update or code change to the package label Mar 14, 2023
@noamzbr noamzbr enabled auto-merge (squash) March 14, 2023 19:56
@noamzbr noamzbr disabled auto-merge March 14, 2023 19:56
@noamzbr
Copy link
Collaborator

noamzbr commented Mar 14, 2023

Hi @mglowacki100, thanks for the contribution! Could you please fix the minor indentation issue raised by pylint? You can validate locally that it's fixed by running make pylint

Fix indentation bug
@mglowacki100
Copy link
Contributor Author

Hi @noamzbr , sorry for this indention bug, I've checked with pylint and updated PR seems to be ok (there are warnings about exception, but it is not caused by my change).

(py39NEW) C:\Users\mglowacki\Documents\GitHub\deepchecks\deepchecks\utils>pylint array_math.py
************* Module C:\Users\mglowacki\Documents\GitHub\deepchecks\.pylintrc
C:\Users\mglowacki\Documents\GitHub\deepchecks\.pylintrc:1:0: E0015: Unrecognized option found: files-output, no-space-check (unrecognized-option)
pylint: Command line or configuration file:1: UserWarning: Specifying exception names in the overgeneral-exceptions option without module name is deprecated and support for it will be removed in pylint 3.0. Use fully qualified name (maybe 'builtins.StandardError' ?) instead.
pylint: Command line or configuration file:1: UserWarning: Specifying exception names in the overgeneral-exceptions option without module name is deprecated and support for it will be removed in pylint 3.0. Use fully qualified name (maybe 'builtins.Exception' ?) instead.
pylint: Command line or configuration file:1: UserWarning: Specifying exception names in the overgeneral-exceptions option without module name is deprecated and support for it will be removed in pylint 3.0. Use fully qualified name (maybe 'builtins.BaseException' ?) instead.

------------------------------------
Your code has been rated at 10.00/10

@noamzbr noamzbr enabled auto-merge (squash) March 15, 2023 06:40
auto-merge was automatically disabled March 15, 2023 08:30

Head branch was pushed to by a user without write access

@noamzbr noamzbr enabled auto-merge (squash) March 15, 2023 08:33
@mglowacki100
Copy link
Contributor Author

Hi @noamzbr I've fixed import order with isort but I've no idea what causes issues with gpu-tests, regarding coverage is there a test that I can repurpose it to increase it (if this is necessary).

@noamzbr
Copy link
Collaborator

noamzbr commented Mar 15, 2023

It's run on an external machine, let me try to just re-run it (could be some connection issue perhaps?)

@noamzbr noamzbr merged commit 1362c79 into deepchecks:main Mar 15, 2023
18 checks passed
@noamzbr
Copy link
Collaborator

noamzbr commented Mar 20, 2023

@all-contributors please add @mglowacki100 for code

@allcontributors
Copy link
Contributor

@noamzbr

I've put up a pull request to add @mglowacki100! 🎉

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature Feature update or code change to the package
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[BUG] custom model wrapper documentation: classes_ property is missing
3 participants