FIX: show bars when the first location is nan #23751
Conversation
Due to the way we handle units on the bar width having an invalid value in the first position of the x bar (of y of barh) would effectively poison all of the widths making all of the bars invisible. This also renames the cbook function _safe_first_non_none function -> _safe_first_finite and adjusts the behavior to also drop nans closes matplotlib#23687
lib/matplotlib/cbook/__init__.py
Outdated
|
|
||
|
|
||
| def _safe_first_non_none(obj, skip_none=True): | ||
| def _safe_first_finite(obj, *, skip_nonfinite=True): | ||
| """ | ||
| Return the first non-None element in *obj*. |
There was a problem hiding this comment.
Docstring needs to be adapted.
lib/matplotlib/cbook/__init__.py
Outdated
| return next(val for val in obj if val is not None) | ||
| return next( | ||
| val for val in obj | ||
| if val is not None and safe_isfinite(val) |
There was a problem hiding this comment.
Logically, should we absorb the None check in safe_isfinite? -I think None is not finite.
There was a problem hiding this comment.
I think we need to check this here:
In [62]: np.isfinite(None)
---------------------------------------------------------------------------
TypeError Traceback (most recent call last)
Cell In [63], line 1
----> 1 np.isfinite(None)
TypeError: ufunc 'isfinite' not supported for the input types, and the inputs could not be safely coerced to any supported types according to the casting rule ''safe''
There was a problem hiding this comment.
Yes, we need the check. I was suggesting that it could logically belong inside safe_isfinite (by the argument that None is not a finite number, similar to NaN).
i.e.
def safe_isfinite(val):
if val is None:
return False
...
and here next(val for val in obj if safe_isfinite(val))
I just realized that this is an inner function, so it's not too important, but I'd still have a slight preference. I'll approve and you can decide whether you want to change it or leave it.
There was a problem hiding this comment.
I agree that is nicer.
|
Will this break the (somewhat edge) case where the user passes in an array with only NaTs (not-a-time) and expect the plot to be configured to use datetimes? |
lib/matplotlib/cbook/__init__.py
Outdated
| return next(val for val in obj if val is not None) | ||
| return next( | ||
| val for val in obj | ||
| if val is not None and safe_isfinite(val) |
There was a problem hiding this comment.
Yes, we need the check. I was suggesting that it could logically belong inside safe_isfinite (by the argument that None is not a finite number, similar to NaN).
i.e.
def safe_isfinite(val):
if val is None:
return False
...
and here next(val for val in obj if safe_isfinite(val))
I just realized that this is an inner function, so it's not too important, but I'd still have a slight preference. I'll approve and you can decide whether you want to change it or leave it.
ce5d274 to
bbf0cd2
Compare
…751-on-v3.6.x Backport PR #23751 on branch v3.6.x (FIX: show bars when the first location is nan)
PR Summary
Due to the way we handle units on the bar width having an invalid value in the
first position of the x bar (of y of barh) would effectively poison all of the
widths making all of the bars invisible.
This also renames the cbook function _safe_first_non_none function ->
_safe_first_finite and adjusts the behavior to also drop nans
closes #23687
PR Checklist
Tests and Styling
pytestpasses).flake8-docstringsand runflake8 --docstring-convention=all).