Uh oh!
There was an error while loading.Please reload this page.
- Notifications
You must be signed in to change notification settings - Fork7.9k
Check for float values for min/max values to ax{v,h}line#17822
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
Uh oh!
There was an error while loading.Please reload this page.
Conversation
I don't think checking for the ability to cast to float is enough, we need to make sure that the data will not select a converter. |
I found |
On master importmatplotlib.pyplotaspltimportmatplotlib.datesasmdatesline=plt.axvline(x=.5,ymin=mdates.num2date(1),ymax=mdates.num2date(1.05))line.set_clip_on(False)plt.show() already errors out: So is the point of this PR to just make the error more explicit? If so, I don't think it needs an API change or note, because I guess that has already occurred. |
That specific example broke ina793bde, which is a date-specific change. Is it still true that other unit-ful values are currently broken? |
Yes, I think it might just be a better error now, with the following example importmatplotlib.pyplotaspltimportastropy.unitsasufromastropy.visualizationimportquantity_supportquantity_support()fig,ax=plt.subplots()ax.scatter(1*u.m,1*u.s)ax.axhline(1*u.s,xmin=0.1*u.m)plt.show() The error before was Traceback (mostrecentcalllast):File"/Users/dstansby/github/matplotlib/lib/matplotlib/axis.py",line1524,inconvert_unitsret=self.converter.convert(x,self.units,self)File"/Users/dstansby/github/astropy/astropy/visualization/units.py",line102,inconvertreturn [v.to_value(unit)forvinval]File"/Users/dstansby/github/astropy/astropy/visualization/units.py",line102,in<listcomp>return [v.to_value(unit)forvinval]AttributeError:'int'objecthasnoattribute'to_value'Theaboveexceptionwasthedirectcauseofthefollowingexception:Traceback (mostrecentcalllast):File"test.py",line8,in<module>ax.axhline(1*u.s,xmin=0.1*u.m)File"/Users/dstansby/github/matplotlib/lib/matplotlib/axes/_axes.py",line847,inaxhlineself.add_line(l)File"/Users/dstansby/github/matplotlib/lib/matplotlib/axes/_base.py",line1977,inadd_lineself._update_line_limits(line)File"/Users/dstansby/github/matplotlib/lib/matplotlib/axes/_base.py",line1999,in_update_line_limitspath=line.get_path()File"/Users/dstansby/github/matplotlib/lib/matplotlib/lines.py",line1011,inget_pathself.recache()File"/Users/dstansby/github/matplotlib/lib/matplotlib/lines.py",line652,inrecachexconv=self.convert_xunits(self._xorig)File"/Users/dstansby/github/matplotlib/lib/matplotlib/artist.py",line175,inconvert_xunitsreturnax.xaxis.convert_units(x)File"/Users/dstansby/github/matplotlib/lib/matplotlib/axis.py",line1526,inconvert_unitsraisemunits.ConversionError('Failed to convert value(s) to axis 'matplotlib.units.ConversionError:Failedtoconvertvalue(s)toaxisunits: [<Quantity0.1m>,1] And the error with this PR is Traceback (mostrecentcalllast):File"test.py",line8,in<module>ax.axhline(1*u.s,xmin=0.1*u.m)File"/Users/dstansby/github/matplotlib/lib/matplotlib/axes/_axes.py",line834,inaxhlineself._check_no_units([xmin,xmax], ['xmin','xmax'])File"/Users/dstansby/github/matplotlib/lib/matplotlib/axes/_axes.py",line926,in_check_no_unitsraiseValueError(f"{name} must be a single scalar value, "ValueError:xminmustbeasinglescalarvalue,butgot0.1m |
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.
I think this is an improved error message.
Uh oh!
There was an error while loading.Please reload this page.
Uh oh!
There was an error while loading.Please reload this page.
Co-authored-by: Elliott Sales de Andrade <quantum.analyst@gmail.com>
Hmm, those exception lines are too long; I wonder why |
Fixes#12198. This now errors if a non-float value is provided for xmin/xmax or ymin/ymax to ax{v,h}{line,span}. This should get an API change or a what's new, but I'm not sure which, could someone advise?