8000 ENH Better error message for check_scalar by thomasjpfan · Pull Request #22218 · scikit-learn/scikit-learn · GitHub
[go: up one dir, main page]
Skip to content

ENH Better error message for check_scalar #22218

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 13 commits into from
Feb 11, 2022
Merged

ENH Better error message for check_scalar #22218

merged 13 commits into from
Feb 11, 2022

Conversation

thomasjpfan
Copy link
Member

Reference Issues/PRs

Follow up to #20724

What does this implement/fix? Explain your changes.

Adjusts the error message to only include the class name and not the <class _> part of the str representation

CC @glemaitre

@glemaitre
Copy link
Member

It might be better understood to say integer and floating instead of number.Integral and number.Real.

@thomasjpfan
Copy link
Member Author

It might be better understood to say integer and floating instead of number.Integral and number.Real.

Is the only reason we use numbers.Integral + numbers.Real is to support np.int64 and friends?

@glemaitre
Copy link
Member

I think yes.

@thomasjpfan
Copy link
Member Author

I updated this PR to use int and float instead of numbers.Integral and numbers.Real.

glemaitre
glemaitre approved these changes Jan 27, 2022
View reviewed changes
Copy link
Member
@glemaitre glemaitre left a comment

Choose a reason for hiding this comment

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

LGTM. Since this is something that we discussed with @ogrisel, I just wanted to be sure that this is in line with what he was proposing.

@thomasjpfan thomasjpfan mentioned this pull request Feb 9, 2022
Merged
lorentzenchr
lorentzenchr approved these changes Feb 9, 2022
View reviewed changes
Copy link
Member
8000
@lorentzenchr lorentzenchr left a comment

Choose a reason for hiding this comment

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

I like it.

@glemaitre glemaitre merged commit 4ff0767 into scikit-learn:main Feb 11, 2022
thomasjpfan added a commit to thomasjpfan/scikit-learn that referenced this pull request Mar 1, 2022
@thomasjpfan @glemaitre
ENH Better error message for check_scalar (scikit-learn#22218)
57aa403 
Co-authored-by: Guillaume Lemaitre <g.lemaitre58@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@glemaitre glemaitre glemaitre approved these changes

@jjerphan jjerphan jjerphan approved these changes

@lorentzenchr lorentzenchr lorentzenchr approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@thomasjpfan @glemaitre @jjerphan @lorentzenchr
0