You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
{{ message }}
Repository navigation
[Bug]: TypeError when plotting against list of datetime.date where 0th element of list is None #23580
Traceback (most recent call last):
File "pyplot_test.py", line 17, in <module>
plt.plot(x, y)
File "<python_path>\lib\site-packages\matplotlib\pyplot.py", line 2769, in plot
return gca().plot(
File "<python_path>\lib\site-packages\matplotlib\axes\_axes.py", line 1634, in plot
self.add_line(line)
File "<python_path>\lib\site-packages\matplotlib\axes\_base.py", line 2288, in add_line
self._update_line_limits(line)
File "<python_path>\lib\site-packages\matplotlib\axes\_base.py", line 2311, in _update_line_limits
path = line.get_path()
File "<python_path>\lib\site-packages\matplotlib\lines.py", line 999, in get_path
self.recache()
File "<python_path>\lib\site-packages\matplotlib\lines.py", line 652, in recache
x = _to_unmasked_float_array(xconv).ravel()
File "<python_path>\lib\site-packages\matplotlib\cbook\__init__.py", line 1298, in _to_unmasked_float_array
return np.asarray(x, float)
File "<python_path>\lib\site-packages\numpy\core\_asarray.py", line 85, in asarray
return array(a, dtype, copy=False, order=order)
TypeError: float() argument must be a string or a number, not 'datetime.date'
Unfortunately I think this is the expected behavior (I was actually a bit surprised it worked with Nones at all). It works because in the time handling code paths we delegate most of the work to numpy and it will cast None to NaT :
The reason this fails with None as the first element is that we do not correctly identify the units (and hence which handlers to use). For the automatic unit detection (we handle datetime through the unit system because time is a number that has time-like units attached to it) we look at the first element (because we assume that the input lists are homogeneous).
We could add a special case to that dispatch to keep walking down the sequence if a None is found. On one hand that would fix this issue and smooth out the rough edge identified here. On the other hand it makes that code even more complex.
I could also see a case for exploding a lot earlier if there is a None in the data, but that is just doubling down on a failure mode that depends on the location in the input of a bad value which is less than ideal.
In the short term, the user-side fix is to use np.array(..., dytpe=np.datetime64) before you pass the values to Matplotlib which ensure that the first element is always a type that will go down the correct unit dispatch.
diff --git i/lib/matplotlib/cbook/__init__.py w/lib/matplotlib/cbook/__init__.py
index b9d5f5eb26..9cbf273259 100644
--- i/lib/matplotlib/cbook/__init__.py+++ w/lib/matplotlib/cbook/__init__.py@@ -1691,19 +1691,12 @@ def safe_first_element(obj):
This is an type-independent way of obtaining the first element, supporting
both index access and the iterator protocol.
"""
- if isinstance(obj, collections.abc.Iterator):- # needed to accept `array.flat` as input.- # np.flatiter reports as an instance of collections.Iterator- # but can still be indexed via [].- # This has the side effect of re-setting the iterator, but- # that is acceptable.- try:- return obj[0]- except TypeError:- pass- raise RuntimeError("matplotlib does not support generators "- "as input")- return next(iter(obj))+ if isinstance(obj, np.flatiter):+ return obj[0]+ elif isinstance(obj, collections.abc.Iterator):+ raise RuntimeError("matplotlib does not support generators as input")+ else:+ return next(val for val in obj if val is not None)
def sanitize_sequence(data):
seems reasonable enough to me? (except that the function becomes a bit misnamed, but such is life).
This assumes that the goal of the first cutout is indeed just to support flatiter (which seems to be the intent of #6712).
None working elsewhere in the list seems to me an implementation accident rather than something we purposely designed. I'm not aware of us using None to mean no data elsewhere in the library (but I could be wrong) so I am not sure we should special case it here.
I think this is an "in for a penny, in for a pound" situation. We are very permissive in terms of what we take as input and currently do take lists of datetimes with None anywhere but the first element. To be consistent we either need to lock down what we take as input more (which is not really feasible) or take this extra complexity.
I'm leaning towards "take the complexity" because that is our job.
@MikiPWata Yes, just be aware we do not consider an issue "claimed" until there is a PR and that there is not complete consensus among the devs on this yet.
Finally passed all checks!
I was wondering if I should keep using the original methods(safe_first_element()) or the _safe_non_none() for all tests that uses safe_first_element.
I am pretty sure that I should be using the new method for the test made for this issue.
Activity
tacaswell commentedon Aug 7, 2022
Unfortunately I think this is the expected behavior (I was actually a bit surprised it worked with
Nones at all). It works because in the time handling code paths we delegate most of the work to numpy and it will castNonetoNaT:The reason this fails with
Noneas the first element is that we do not correctly identify the units (and hence which handlers to use). For the automatic unit detection (we handle datetime through the unit system because time is a number that has time-like units attached to it) we look at the first element (because we assume that the input lists are homogeneous).We could add a special case to that dispatch to keep walking down the sequence if a
Noneis found. On one hand that would fix this issue and smooth out the rough edge identified here. On the other hand it makes that code even more complex.I could also see a case for exploding a lot earlier if there is a
Nonein the data, but that is just doubling down on a failure mode that depends on the location in the input of a bad value which is less than ideal.In the short term, the user-side fix is to use
np.array(..., dytpe=np.datetime64)before you pass the values to Matplotlib which ensure that the first element is always a type that will go down the correct unit dispatch.anntzer commentedon Aug 7, 2022
seems reasonable enough to me? (except that the function becomes a bit misnamed, but such is life).
This assumes that the goal of the first cutout is indeed just to support flatiter (which seems to be the intent of #6712).
jklymak commentedon Aug 7, 2022
None working elsewhere in the list seems to me an implementation accident rather than something we purposely designed. I'm not aware of us using None to mean no data elsewhere in the library (but I could be wrong) so I am not sure we should special case it here.
tacaswell commentedon Aug 7, 2022
I think this is an "in for a penny, in for a pound" situation. We are very permissive in terms of what we take as input and currently do take lists of datetimes with
Noneanywhere but the first element. To be consistent we either need to lock down what we take as input more (which is not really feasible) or take this extra complexity.I'm leaning towards "take the complexity" because that is our job.
tacaswell commentedon Aug 7, 2022
I'm labeling this as a good first issue because we have a plausbile looking patch and there is no (explicit) new public API.
Steps:
MikiPWata commentedon Aug 7, 2022
Hi there,
Can I work on this?
tacaswell commentedon Aug 7, 2022
@MikiPWata Yes, just be aware we do not consider an issue "claimed" until there is a PR and that there is not complete consensus among the devs on this yet.
jklymak commentedon Aug 7, 2022
I checked that None is also ignored in bare lists of floats, so I guess this is OK. We probably should document this somehow though...
MikiPWata commentedon Aug 9, 2022
@jklymak
my PR is pretty much good to go(passed all checks), so would love to have a review. Thank you in advance!
tacaswell commentedon Aug 9, 2022
well, we finally had two people take up a "good first issue" at about the same time.
MikiPWata commentedon Aug 11, 2022
Is there anything I can/should do to fix the test failures from CI?
MikiPWata commentedon Aug 11, 2022
Finally passed all checks!
I was wondering if I should keep using the original methods(safe_first_element()) or the _safe_non_none() for all tests that uses safe_first_element.
I am pretty sure that I should be using the new method for the test made for this issue.
timhoffm commentedon Aug 11, 2022
Closed by #23587.