Repository navigation
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 our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from 1 commit
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
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
- Loading branch information
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -1703,10 +1703,10 @@ def safe_first_element(obj): | |
| This is an type-independent way of obtaining the first element, | ||
| supporting both index access and the iterator protocol. | ||
| """ | ||
| return _safe_first_non_none(obj, skip_none=False) | ||
| return _safe_first_finite(obj, skip_nonfinite=False) | ||
|
|
||
|
|
||
| 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*. | ||
| This is a method for internal use. | ||
|
|
@@ -1715,7 +1715,14 @@ def _safe_first_non_none(obj, skip_none=True): | |
| supporting both index access and the iterator protocol. | ||
| The first non-None element will be obtained when skip_none is True. | ||
| """ | ||
| if skip_none is False: | ||
| def safe_isfinite(val): | ||
| try: | ||
| return np.isfinite(val) if np.isscalar(val) else True | ||
|
timhoffm marked this conversation as resolved.
|
||
| except TypeError: | ||
| # This is something that numpy can not make heads or tails | ||
| # of, assume "finite" | ||
| return True | ||
| if skip_nonfinite is False: | ||
| if isinstance(obj, collections.abc.Iterator): | ||
| # needed to accept `array.flat` as input. | ||
| # np.flatiter reports as an instance of collections.Iterator | ||
|
|
@@ -1730,12 +1737,16 @@ def _safe_first_non_none(obj, skip_none=True): | |
| "as input") | ||
| return next(iter(obj)) | ||
| elif isinstance(obj, np.flatiter): | ||
| # TODO do the finite filtering on this | ||
| 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) | ||
| return next( | ||
| val for val in obj | ||
| if val is not None and safe_isfinite(val) | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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.
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think we need to check this here:
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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 inside i.e. and here 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.
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I agree that is nicer. |
||
| ) | ||
|
|
||
|
|
||
| def sanitize_sequence(data): | ||
|
|
||
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.