Skip to content

Add ishistogammed option to the hist plotting method #12306

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
Closed
wants to merge 1 commit into from
Closed

Add ishistogammed option to the hist plotting method #12306

wants to merge 1 commit into from

Conversation

shane-breeze
Copy link

PR Summary

The option ishistogrammed to the hist plotting method, e.g.

plt.hist(x, bins=bins, ishistogrammed=True)

This is used if x is already histogrammed through the np.histogram function (or similar).

Use case:

  • If the non-histogrammed x is too large to pass to plt.hist in one go then the user can histogram in chunks themselves and then pass the summed histograms to do the plotting.

PR Checklist

  • Has Pytest style unit tests
  • Code is Flake 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

anntzer
anntzer requested changes Sep 27, 2018
View reviewed changes
Copy link
Contributor

@anntzer anntzer left a comment •
edited
Loading

Choose a reason for hiding this comment

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

I'd rather you just use plt.bar instead... no?
(Yes, you will have to perform some additional calculations yourself if you want e.g. to get a cumulative histogram, but if you already went through the bother of doing the histogramming then an additional cumsum() is not too hard.)

Feel free to dismiss this review if there's consensus to do so.

@shane-breeze
Copy link
Author

I'm also using the stacked options in plt.hist with my own histogrammed data inputs. It's a lot more convenient to use plt.hist for this rather than plt.bar, which requires several calls working out the bottoms at each point. Implementing a stacked-like option in plt.bar doesn't make as much sense to me but I'm open to suggestions

@jklymak
Copy link
Member

jklymak commented Sep 27, 2018

My knee-jerk reaction is that I'm also against overloading hist for this. Maybe refactor the cumulative bar-making machinery in hist to a new method cumulativebar? Or add the functionality to bar itself if easily done.

@ImportanceOfBeingErnest
Copy link
Member

ImportanceOfBeingErnest commented Sep 27, 2018 •
edited
Loading

Looks like because bar does not have a stacked option (or more generally does not allow for multidimensional input) you want to misuse hist to draw the bar graph instead.
There were suggestions on creating a bars or barplots function in the past, so I'd rather follow up on those:
There is #11048 which I fear everyone just forgot about. It seemed quite complete last time I checked, but now lost most of its content. Still the discussion is quite valuable.

@shane-breeze
Copy link
Author

#11048 would be useful. Any idea if that PR will continue?

Anyway, I'll close this for now

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@anntzer anntzer anntzer requested changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@shane-breeze @jklymak @ImportanceOfBeingErnest @anntzer