Skip to content

Improve categorical converter error message #17824

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 2 commits into from
Jul 5, 2020
Merged

Improve categorical converter error message #17824

merged 2 commits into from
Jul 5, 2020

Conversation

dstansby
Copy link
Member

@dstansby dstansby commented Jul 2, 2020

PR Summary

xref #11095. Provide a nicer error message if the user tries to set a unit without _mapping to a categorical converter.

PR Checklist

  • Has Pytest style unit tests
  • Code is Flake 8 compliant
  • New features are documented, with examples if plot related
  • Documentation is sphinx and numpydoc compliant
  • Added an entry to doc/users/next_whats_new/ if major new feature (follow instructions in README.rst there)
  • Documented in doc/api/next_api_changes/* if API changed in a backward-incompatible way

@dstansby dstansby added this to the v3.4.0 milestone Jul 2, 2020
@dstansby dstansby mentioned this pull request Jul 2, 2020
Closed
@QuLogic
Copy link
Member

QuLogic commented Jul 2, 2020

It's slightly more specific than an AttributeError, but I'm not sure what the user is supposed to do with this information?

@tacaswell tacaswell mentioned this pull request Jul 3, 2020
Merged
3 tasks
@anntzer
Copy link
Contributor

anntzer commented Jul 3, 2020

I agree the error message should be something more actionable e.g. "not a valid categorical unit" or similar

@dstansby
Copy link
Member Author

dstansby commented Jul 3, 2020

How about now? I think this is a small improvement avoids the user having to work out where and why the attribute error was thrown, immediately saying "the problem was with the unit passed to a categorical converter".

@QuLogic QuLogic requested a review from story645 July 3, 2020 21:26
QuLogic
QuLogic reviewed Jul 3, 2020
View reviewed changes
story645
story645 approved these changes Jul 5, 2020
View reviewed changes
Copy link
Member

@story645 story645 left a comment

Choose a reason for hiding this comment

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

Slightly concerned since ._mapping is a private attribute so I don't quite know what users can do to fix this error.

@tacaswell tacaswell merged commit 61c0d4a into matplotlib:master Jul 5, 2020
@tacaswell
Copy link
Member

It is at least a less-confusing error message that the type error (for the same private attribute) that they are getting now.

@dstansby dstansby deleted the cat-conv-none branch July 5, 2020 21:32
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@QuLogic QuLogic QuLogic left review comments

@efiring efiring efiring approved these changes

@tacaswell tacaswell tacaswell approved these changes

@story645 story645 story645 approved these changes

Assignees
No one assigned
Labels
topic: categorical topic: units and array ducktypes
Projects
None yet
Milestone
v3.4.0
Development

Successfully merging this pull request may close these issues.
6 participants
@dstansby @QuLogic @anntzer @tacaswell @efiring @story645