Skip to content

[ENH]: Use repr instead of str in the error message #21959

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
Closed
e5f6bd opened this issue Dec 15, 2021 · 9 comments · Fixed by #24403
Closed

[ENH]: Use repr instead of str in the error message #21959

e5f6bd opened this issue Dec 15, 2021 · 9 comments · Fixed by #24403
Labels
Good first issue Open a pull request against these issues if there are no active ones! New feature
Milestone
v3.7.0

Comments

@e5f6bd
Copy link

e5f6bd commented Dec 15, 2021 •
edited
Loading

Problem

I mistakenly supplied "blue\n" as the argument c for matplotlib.axes.Axes.scatter , then matplitlib claimed for illegal color name like this:

ValueError: 'c' argument must be a color, a sequence of colors, or a sequence of numbers, not blue

I was not aware that the argument actually contained a trailing newline so I was very confused.

Proposed solution

The error message would be nicer if it outputs user's input via repr.
For example, in this case the error message here can be easily replced with:

                    raise ValueError(
                        f"'c' argument must be a color, a sequence of colors, "
                        f"or a sequence of numbers, not {c!r}") from

so that we may now get an easy-to-troubleshoot error like this:

ValueError: 'c' argument must be a color, a sequence of colors, or a sequence of numbers, not "blue\n"

This kind of improvement can be applied to many other places.
@tacaswell tacaswell added this to the v3.6.0 milestone Dec 15, 2021
@tacaswell tacaswell added the Good first issue Open a pull request against these issues if there are no active ones! label Dec 15, 2021
@tacaswell
Copy link
Member

Labeling this as a good first issue as this is a straight forward change that should not bring up any API design issues (yes technically changing the wording of an error message may break someone, but that is so brittle I am not going to worry about that). Will need a test (the new line example is a good one!).

@tacaswell
Copy link
Member

@e5f6bd If you are interested in opening a PR with this change, please have at it!

@e5f6bd
Copy link
Author

e5f6bd commented Dec 15, 2021

@e5f6bd Thanks, but I'm a bit busy recently, so I'll leave this chance to someone else.

@ghost ghost mentioned this issue Dec 15, 2021
Closed
@Hashcode-Ankit
Copy link

If this issue has not been resolved yet and nobody working on it I would like to try it

@story645
Copy link
Member

story645 commented Dec 29, 2021 •
edited
Loading

Looks like #21968 is addressing this issue

@Hashcode-Ankit
Copy link

yeah but the issue still open

35C4n0r added a commit to 35C4n0r/matplotlib that referenced this issue Jan 6, 2022
@35C4n0r
Issue Fix matplotlib#21959
82aa3a5 
In case of ValueError caused by user given input, raw string will be displayed in Error Message.
35C4n0r added a commit to 35C4n0r/matplotlib that referenced this issue Jan 6, 2022
@35C4n0r
Changes made to _axes.py as suggested in issue matplotlib#21959
769ef80 
Now in case of ValueError caused by user defined input, raw string will be shown in the error message.
This was referenced Mar 27, 2022
Merged
Merged
@oscargus oscargus mentioned this issue Apr 24, 2022
Closed
6 tasks
@Cloverwave
Copy link

Working on this now in axes_.py,
future plans to work on this in all files with error raising.

@Cloverwave
Copy link

Also converting f strings to template strings where I see them in raised errors

@oscargus oscargus mentioned this issue Jul 4, 2022
Closed
6 tasks
@oscargus
Copy link
Member

oscargus commented Jul 4, 2022

Also converting f strings to template strings where I see them in raised errors

f-string are usually preferred due to speed issues. Which is not critical, but if one can choose, f-strings are not worse.

@QuLogic QuLogic modified the milestones: v3.6.0, v3.7.0 Aug 24, 2022
jefromyers added a commit to jefromyers/matplotlib that referenced this issue Nov 8, 2022
@jefromyers
use repr in error message closes matplotlib#21959
7ec1c0a
jefromyers added a commit to jefromyers/matplotlib that referenced this issue Nov 8, 2022
@jefromyers
use repr in error message closes matplotlib#21959
6f1ece0
jefromyers added a commit to jefromyers/matplotlib that referenced this issue Nov 8, 2022
@jefromyers
use repr in error message closes matplotlib#21959
ffb91b3
@jefromyers jefromyers mentioned this issue Nov 8, 2022
Merged
2 tasks
@story645 story645 linked a pull request Nov 8, 2022 that will close this issue
Use repr in error message Addresses #21959 #24403
Merged
2 tasks
QuLogic pushed a commit that referenced this issue Nov 10, 2022
@jefromyers
Use repr in scatter error message (#24403)
04ce3e9 
* use repr in error message closes #21959

* add test for repr error message

* fix formatting

* clean up formatting and naming

* add `pragma: no cover`

* remove import and function name a bit more explicit
melissawm pushed a commit to melissawm/matplotlib that referenced this issue Dec 19, 2022
@jefromyers @melissawm
Use repr in scatter error message (matplotlib#24403)
547460f 
* use repr in error message closes matplotlib#21959

* add test for repr error message

* fix formatting

* clean up formatting and naming

* add `pragma: no cover`

* remove import and function name a bit more explicit
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
Good first issue Open a pull request against these issues if there are no active ones! New feature
Projects
None yet
Milestone
v3.7.0
Development

Successfully merging a pull request may close this issue.

Use repr in error message Addresses #21959 jefromyers/matplotlib
7 participants
@tacaswell @QuLogic @story645 @oscargus @e5f6bd @Hashcode-Ankit @Cloverwave