FazBrowse GitHub Viewer | Trending |
URL:
| Home
Tools: [Download Repo ZIP]   [Original HTTPS Page]

Improve categorical converter error message by dstansby · Pull Request #17824 · matplotlib/matplotlib · GitHub

  1. matplotlib
  2. matplotlib

Repository navigation

Improve categorical converter error message - #17824

Merged
tacaswell merged 2 commits into
matplotlib:masterfrom
dstansby:cat-conv-none
Jul 5, 2020
ConversationCommits (2)ChecksFiles changed
Merged

Improve categorical converter error message#17824
tacaswell merged 2 commits into
matplotlib:masterfrom
dstansby:cat-conv-none

Conversation

dstansby commented Jul 2, 2020

Copy link
Copy Markdown
Member

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 added this to the v3.4.0 milestone Jul 2, 2020
dstansby mentioned this pull request Jul 2, 2020
Closed

QuLogic commented Jul 2, 2020

Copy link
Copy Markdown
Member

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

tacaswell mentioned this pull request Jul 3, 2020
Merged
2 of 3 tasks

anntzer commented Jul 3, 2020

Copy link
Copy Markdown
Contributor

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

dstansby commented Jul 3, 2020

Copy link
Copy Markdown
Member Author

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 requested a review from story645 July 3, 2020 21:26
QuLogic reviewed Jul 3, 2020
View reviewed changes
Comment thread lib/matplotlib/category.py Outdated
story645 approved these changes Jul 5, 2020
View reviewed changes

story645 left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

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

tacaswell merged commit 61c0d4a into matplotlib:master Jul 5, 2020

tacaswell commented Jul 5, 2020

Copy link
Copy Markdown
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 deleted the cat-conv-none branch July 5, 2020 21:32
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters. Learn more about bidirectional Unicode characters
Show hidden characters
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

QuLogic QuLogic left review comments
efiring efiring approved these changes
tacaswell tacaswell approved these changes
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

Back | FazBrowse Home | New Git URL