Improve error message for gridspec when the index is not an integer. #24630
Conversation
There was a problem hiding this comment.
Thank you for opening your first PR into Matplotlib!
If you have not heard from us in a while, please feel free to ping @matplotlib/developers or anyone who has commented on the PR. Most of our reviewers are volunteers and sometimes things fall through the cracks.
You can also join us on gitter for real-time discussion.
For details on testing, writing docs, and our review process, please see the developer guide
We strive to be a welcoming and open project. Please follow our Code of Conduct.
| @@ -599,7 +599,10 @@ def _from_subplot_args(figure, args): | ||
| ) | ||
| i, j = num | ||
| else: | ||
| if not isinstance(num, Integral) or num < 1 or num > rows*cols: |
There was a problem hiding this comment.
I would slightly prefer making the previous error message more complete, e.g. "num must be an integer with 1 <= num <= ..., not ...", instead of delivering the info in a piecemeal fashion. (If I accidentally pass 0.5 this will help me fixing the entire error (both type and value) at once, rather than first slapping an int() call, re-running the code, and seeing the second error.)
There was a problem hiding this comment.
I also have a slight preference for a single more verbose error error message.
|
Thank you and congratulations on your first merged Matplotlib PR @ShawnZhong 🎉 I hope we hear from you again! |
PR Summary
plt.subplot(1, 2, 0.5)raisesValueError: num must be 1 <= num <= 2, not 0.5, which is not so useful.This PR changes it to report
Subplot specifier must be an integer, not 0.5PR Checklist
Documentation and Tests
pytestpasses)Release Notes
.. versionadded::directive in the docstring and documented indoc/users/next_whats_new/.. versionchanged::directive in the docstring and documented indoc/api/next_api_changes/next_whats_new/README.rstornext_api_changes/README.rst