Uh oh!
There was an error while loading.Please reload this page.
- Notifications
You must be signed in to change notification settings - Fork7.9k
FIX: show bars when the first location is nan#23751
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
Due to the way we handle units on the bar width having an invalid value in thefirst position of the x bar (of y of barh) would effectively poison all of thewidths 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 nansclosesmatplotlib#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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
Logically, should we absorb the None check in safe_isfinite? -I think None is not finite.
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 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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
Yes, we need the check. I was suggesting that it could logically belong insidesafe_isfinite (by the argument thatNone is not a finite number, similar to NaN).
i.e.
def safe_isfinite(val): if val is None: return False ...and herenext(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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
I agree that is nicer.
Uh oh!
There was an error while loading.Please reload this page.
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? |
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.
One optional logical improvement.
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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
Yes, we need the check. I was suggesting that it could logically belong insidesafe_isfinite (by the argument thatNone is not a finite number, similar to NaN).
i.e.
def safe_isfinite(val): if val is None: return False ...and herenext(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.
…751-on-v3.6.xBackport 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).