napari-matplotlib
napari-matplotlib copied to clipboard
Add widgets for setting histogram bins
Fixes #239
- added
QDoubleSpinBox
widgets for setting the start and stop edges of the histogram bins (default to the min and max data values, same as existing values used) - added a
QSpinBox
widget for setting the number of bins to use (defaults to 100, same as the existing value used) - add callbacks to re-draw figure when value of widgets change
- ~~set the
dtype
fornp.linspace
when creating bins (so integer bin limits are used for integer images). This caused thetest_histogram_2D
test to fail (as the test layer has integer data but float bins were being used) so I've updated the baseline image~~ - this is now done in #244 - added a test for the
HistogramWidget
when the bin parameters are set from the widgets - add
start
,stop
, andnum_bins
parameters to_get_bins
function so that bin parameters set in the widgets can be used for settings bins - show notification if the bin parameters requested by the user have been modified before plotting (this can happen with integer data as we need to make sure the bins have integer widths)
https://github.com/matplotlib/napari-matplotlib/assets/29753790/876ab0e9-0b46-4efd-b408-c4ffa184d799
Thanks for the PR! Could you put the integer dtype histogram change into a separate PR to make it easier to review?
Could you put the integer dtype histogram change into a separate PR to make it easier to review?
Yep, will do 🙂
I am finally thinking about this (sorry it took so long!), and my current thoughts are to keep things simple we should just have a setting for the number of bins, and show the full range of data without being able to control the start/stop. I am open to adding confiurable start/stop in the future if there's a compelling use case, but for now I think configuring the number of bins is a nice feature that isn't too complicated by itself.
Would you be able to update this PR (or open a new one) to just have the number of bins configurable? Obviously no rush at all 😄, but let me know if it isn't likely to be soon in case I get the bug to implement it
Yeah good idea, it became surprisingly complicated! I've removed the notification of requested vs actual bins that I previously added (you can see it in the video above) , do you want it added back?
Sorry this has taken so long to get to,
no worries 😄 Thanks for the reviews and for merging it!