Skip to content

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

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 1 commit into from
May 24, 2018
Merged

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

merged 1 commit into from
May 24, 2018

Conversation

maahn
Copy link

@maahn maahn commented May 17, 2018 •
edited
Loading

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
Copy link
Member

jklymak commented May 17, 2018

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

@maahn
Copy link
Author

maahn commented May 22, 2018

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
Copy link
Member

@timhoffm timhoffm left a comment

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
Copy link
Author

maahn commented May 24, 2018

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
Copy link
Member

jklymak commented May 24, 2018

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