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

[issue-1248] features inference for passed into check dataframes #1355

Merged
merged 8 commits into from May 3, 2022

Conversation

yromanyshyn
Copy link
Contributor

resolves #1248

@Nadav-Barak
it is enough to modify line 889 in Dataset.cast_to_dataset to solve that issue, but I am not sure how clear the warning message raised by the features inference logic (Dataset._infer_categorical_features line 573) will be for the users in this case. What do you think?

@yromanyshyn yromanyshyn added the feature Feature update or code change to the package label Apr 29, 2022
@yromanyshyn yromanyshyn requested a review from a team as a code owner April 29, 2022 11:37
@yromanyshyn yromanyshyn self-assigned this Apr 29, 2022
@yromanyshyn yromanyshyn requested a review from a team as a code owner April 29, 2022 13:13
@matanper
Copy link
Contributor

matanper commented May 1, 2022

I think that in Dataset line 889 we can add the warning "DataFrame was passed, initializing deepchecks.tabular.Dataset using the given dataframe"

@Nadav-Barak
Copy link
Collaborator

I think the warning massage is good for both cases (if the user passed a DF directly or created a DS without specifying categorical)

tests/base/dataset_test.py Outdated Show resolved Hide resolved
tests/checks/overview/columns_info_test.py Outdated Show resolved Hide resolved
tests/checks/overview/columns_info_test.py Outdated Show resolved Hide resolved
tests/checks/integrity/string_mismatch_comparison_test.py Outdated Show resolved Hide resolved
@yromanyshyn
Copy link
Contributor Author

@matanper

I think that in Dataset line 889 we can add the warning "DataFrame was passed, initializing deepchecks.tabular.Dataset using the given dataframe"

Think it will be redundant, this function is part of a public API and it could be strange for user to see warning as this when he/she will try to use this function directly

@matanper
Copy link
Contributor

matanper commented May 2, 2022

@matanper

I think that in Dataset line 889 we can add the warning "DataFrame was passed, initializing deepchecks.tabular.Dataset using the given dataframe"

Think it will be redundant, this function is part of a public API and it could be strange for user to see warning as this when he/she will try to use this function directly

I don't see a reason for it to be a public API, it doesn't help the user more than calling the ctor

@yromanyshyn
Copy link
Contributor Author

@matanper

I don't see a reason for it to be a public API, it doesn't help the user more than calling the ctor

ok, I have not changed its visibility marker, but I have added a warning

@yromanyshyn yromanyshyn merged commit 59c42ac into main May 3, 2022
@delete-merged-branch delete-merged-branch bot deleted the issue-1248 branch May 3, 2022 12:28
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.

[FEAT] auto infer features when dataframe is passed to check + warning
4 participants