Uh oh!
There was an error while loading.Please reload this page.
- Notifications
You must be signed in to change notification settings - Fork8.1k
Improving error message for width and position type mismatch in violinplot#30752
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.
By clicking “Sign up for GitHub”, you agree to ourterms of service andprivacy statement. We’ll occasionally send you account related emails.
Already on GitHub?Sign in to your account
base:main
Are you sure you want to change the base?
Conversation
| defviolin_plot_stats(): | ||
| datetimes= [ | ||
| datetime.datetime(2023,2,10), | ||
| datetime.datetime(2023,5,18), | ||
| datetime.datetime(2023,6,6) | ||
| ] | ||
| return [{ | ||
| 'coords':datetimes, | ||
| 'vals': [0.1,0.5,0.2], | ||
| 'mean':datetimes[1], | ||
| 'median':datetimes[1], | ||
| 'min':datetimes[0], | ||
| 'max':datetimes[-1], | ||
| 'quantiles':datetimes | ||
| }, { | ||
| 'coords':datetimes, | ||
| 'vals': [0.2,0.3,0.4], | ||
| 'mean':datetimes[2], | ||
| 'median':datetimes[2], | ||
| 'min':datetimes[0], | ||
| 'max':datetimes[-1], | ||
| 'quantiles':datetimes | ||
| }] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
This PR handles date aspects on x-axis-related quantities. The stats refer to the y-axis and their unit does not matter in this PR. For simplicity and clarity, let's not use dates for them.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
return [{ 'coords': datetimes, 'vals': [0.1, 0.5, 0.2], 'mean': datetimes[1], 'median': datetimes[1], 'min': datetimes[0], 'max': datetimes[-1], 'quantiles': datetimesThe quantities are the y-axis values- that, and the mean, median, min and max values for the quantities spread is what you are asking me to change? Just wanted to confirm
Something like this:
'coords': datetimes, 'vals': [0.1, 0.5, 0.2], 'mean': 0.5, 'median': 0.5, 'min': 0.1, 'max': 0.2, 'quantiles': [0.1, 0.5, 0.2]There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
Yes, but please do not use obviously nonsensical data (like max < mean).
lib/matplotlib/tests/test_axes.py Outdated
| 'vals': [0.1,0.5,0.2], | ||
| 'mean':0.5, | ||
| 'median':0.5, | ||
| 'min':0.1, | ||
| 'max':0.2, | ||
| 'quantiles': [0.1,0.5,0.2] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
Please fix the statistics also here.
This PR intends to close [ENH]: Support using datetimes as positions argument to violin(...)#30417
It should be possible to set the position of a violin plot to be a datetime. Currently, an attempt to do this results in this error message: TypeError: unsupported operand type(s) for +: 'float' and 'datetime.datetime'
The error stems from trying to perform operations between float and datetime if datetime was provided as position arguments.
The proposed solution improves the error message to be:
"If positions are datetime/date values, pass widths as datetime.timedelta (e.g., datetime.timedelta(days=10)) or numpy.timedelta64.
unit tests are in tests\test_violinplot_datetime.py
I had opened another PR#30545, but messed up the commits while making changes. I am making this one after reading the suggestion here:#30508 (comment) by@rcomer . This change updates the error message instead of converting the position and width