Skip to content

FIX: allow nan values in data for plt.hist#11267

Merged
jklymak merged 1 commit into
matplotlib:masterfrom
maahn:fixHistNan
May 24, 2018
Merged

FIX: allow nan values in data for plt.hist#11267
jklymak merged 1 commit into
matplotlib:masterfrom
maahn:fixHistNan

Conversation

@maahn

@maahn maahn commented May 17, 2018 •
edited
Loading

Copy link
Copy Markdown

PR Summary

This change is a little older, but somehow made it never into a PR. Simple patch to allow plt.hist to determine the range even if there are nan's in the the data, e.g. plt.hist([1,2,3,np.nan]). Currently ValueError: max must be larger than min in range parameter. is raised.

PR Checklist

  • Has Pytest style unit tests
  • Code is PEP 8 compliant
  • New features are documented, with examples if plot related
  • Documentation is sphinx and numpydoc compliant
  • Added an entry to doc/users/next_whats_new/ if major new feature (follow instructions in README.rst there)
  • Documented in doc/api/api_changes.rst if API changed in a backward-incompatible way

@jklymak

jklymak commented May 17, 2018

Copy link
Copy Markdown
Member

See #9706 for a longer discussion of this. However, it looks like it is still an open issue.

@maahn

maahn commented May 22, 2018

Copy link
Copy Markdown
Author

Thanks for the reference, thats for sure a more thorough solution. But maybe its worth nevertheless to apply the patch here as long you don't decide about the other one, because plt.hist can already handle nans in a histogram, the only thing failing is the determination of range, i.e. plt.hist([1,2,3,np.nan],range=(1,3)) works. So I would consider this more a bugfix than a missing feature.

timhoffm
timhoffm approved these changes May 22, 2018
View reviewed changes

@timhoffm timhoffm left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Approving as a bug fix for more consistency. If plt.hist works with nans and an explicit range, it should also work without a range.

Not saying anything about how missing data should be treated more generally, but since #9706 doesn't appear to advance, this solution is good enough for the moment.

@maahn

maahn commented May 24, 2018

Copy link
Copy Markdown
Author

Thanks, it looks like CircleCI failed due to some unrelated hiccup on their side

ERROR:   py27: commands failed
ERROR:   py34: InvocationError for command /home/ubuntu/matplotlib/.tox/py34/bin/pip install /home/ubuntu/matplotlib/.tox/dist/matplotlib-2.0.0b1+12075.g11cecfe.zip (see /home/ubuntu/matplotlib/.tox/py34/log/py34-2.log) (exited with code 1)
ERROR:   py35: Error creating virtualenv. Note that some special characters (e.g. ':' and unicode symbols) in paths are not supported by virtualenv. Error details: InvocationError("Failed to get version_info for python3.5: pyenv: python3.5: command not found\n\nThe `python3.5' command exists in these Python versions:\n  3.5.0\n\n", None)

Do you just ignore that? Or can you trigger that manually again?

@jklymak jklymak merged commit 103d839 into matplotlib:master May 24, 2018
@jklymak

jklymak commented May 24, 2018

Copy link
Copy Markdown
Member

I think the circle-ci was just down... Since this has no documentation issues, we can ignore that test...

@tacaswell tacaswell added this to the v3.0 milestone May 28, 2018
This was referenced Mar 31, 2019
Closed
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@jklymak jklymak jklymak approved these changes

@timhoffm timhoffm timhoffm approved these changes

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

v3.0.0

Development

Successfully merging this pull request may close these issues.

4 participants

@maahn @jklymak @timhoffm @tacaswell