Skip to content

ENH: Improve error mesage verbosity in string accessor #59900

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 8 commits into from
Oct 28, 2024
Merged

ENH: Improve error mesage verbosity in string accessor #59900

merged 8 commits into from
Oct 28, 2024

Conversation

KevsterAmp
Copy link
Contributor

Continued stale PR: #59649

@KevsterAmp
Copy link
Contributor Author

@rhshadrach - could you take a look at this? thanks

rhshadrach
rhshadrach requested changes Oct 21, 2024
View reviewed changes
Copy link
Member

@rhshadrach rhshadrach left a comment

Choose a reason for hiding this comment

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

Looking good - if this isn't currently hit by tests, they should be added.

pandas/core/strings/accessor.py Outdated
@@ -255,7 +255,9 @@ def _validate(data):
inferred_dtype = lib.infer_dtype(values, skipna=True)

if inferred_dtype not in allowed_types:
raise AttributeError("Can only use .str accessor with string values!")
raise AttributeError(
f"Can only use .str accessor with string values!, not {inferred_dtype}"
Copy link
Member

Choose a reason for hiding this comment

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

Can you remove the !, I believe it is not grammatically correct.

@KevsterAmp KevsterAmp requested a review from rhshadrach October 22, 2024 00:52
rhshadrach
rhshadrach requested changes Oct 22, 2024
View reviewed changes
Copy link
Member

@rhshadrach rhshadrach left a comment

Choose a reason for hiding this comment

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

Looks good! Can you update the test test_str_attribute in tests/series/accessors/test_str_accessor.py. Right now it is checking this case, but only checks part of the message. I think we can just update that to test the full message for this.

@KevsterAmp KevsterAmp requested a review from rhshadrach October 24, 2024 13:27
@rhshadrach
Copy link
Member

I think this looks good, but the CI has many unrelated failures. I'd like to get those resolved before merging PRs.

@rhshadrach
Copy link
Member

CI is fixed, can you merge main.

@KevsterAmp
Copy link
Contributor Author

@rhshadrach - CI looks good. Thanks

rhshadrach
rhshadrach approved these changes Oct 28, 2024
View reviewed changes
Copy link
Member

@rhshadrach rhshadrach left a comment

Choose a reason for hiding this comment

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

lgtm

@rhshadrach rhshadrach added Strings String extension data type and string data Enhancement Error Reporting Incorrect or improved errors from pandas labels Oct 28, 2024
@rhshadrach rhshadrach changed the title Improve error mesage verbosity ENH: Improve error mesage verbosity Oct 28, 2024
@rhshadrach rhshadrach changed the title ENH: Improve error mesage verbosity ENH: Improve error mesage verbosity in string accessor Oct 28, 2024
@rhshadrach rhshadrach added this to the 3.0 milestone Oct 28, 2024
@rhshadrach rhshadrach merged commit e3a3a4a into pandas-dev:main Oct 28, 2024
62 of 63 checks passed
@rhshadrach
Copy link
Member

Thanks @KevsterAmp

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@rhshadrach rhshadrach rhshadrach approved these changes

Assignees
No one assigned
Labels
Enhancement Error Reporting Incorrect or improved errors from pandas Strings String extension data type and string data
Projects
None yet
Milestone
3.0
Development

Successfully merging this pull request may close these issues.

ENH: increase verbosity on error
2 participants
@KevsterAmp @rhshadrach