Skip to content

Add ishistogammed option to the hist plotting method#12306

Closed
shane-breeze wants to merge 1 commit intomatplotlib:masterfrom
shane-breeze:hist-ishistogrammed-option
Closed

Add ishistogammed option to the hist plotting method#12306
shane-breeze wants to merge 1 commit intomatplotlib:masterfrom
shane-breeze:hist-ishistogrammed-option

Conversation

@shane-breeze
Copy link
Copy Markdown

@shane-breeze shane-breeze commented Sep 27, 2018

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

shane-breeze commented Sep 27, 2018

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

shane-breeze commented Sep 27, 2018

#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