vmin=0 in 2d histograms #157
Conversation
anesthetic/plot.py
Outdated
| @@ -787,7 +788,8 @@ def hist_plot_2d(ax, data_x, data_y, *args, **kwargs): | |||
| pdf[pdf < cmin] = np.ma.masked | |||
| if cmax is not None: | |||
| pdf[pdf > cmax] = np.ma.masked | |||
| image = ax.pcolormesh(x, y, pdf.T, cmap=cmap, vmin=0, vmax=pdf.max(), | |||
| image = ax.pcolormesh(x, y, pdf.T, cmap=cmap, | |||
| vmin=vmin, vmax=pdf.max(), | |||
There was a problem hiding this comment.
Do we actually need the vmax=pdf.max() here? Could we remove it such that if someone wanted to fix it for some reason this would be possible?
If we do need it, do we also need it for ax.hist2d further up?
There was a problem hiding this comment.
Would it make sense to normalize the histogram like in the 1d case and set vmax=1 by default? That would make the maximum consistent across a triangle plot (keeping in mind that in most cases there won't be a colorbar to go alongside the plot).
@Stefan-Heimersheim, @williamjameshandley: Thoughts?
There was a problem hiding this comment.
vmax=pdf.max()
This seems to be the default in pcolormesh
vmin, vmax: float, default: None
The colorbar range. If None, suitable min/max values are automatically chosen by the Normalize instance (defaults to the respective min/max values of C in case of the default linear scaling). It is deprecated to use vmin/vmax when norm is given.
So I think we can just leave the default if it doesn't do anything. Edit: Same for hist2d via Normalize
Would it make sense to normalize the histogram like in the 1d case and set vmax=1 by default?
Doesn't this interfere with the y-axis in double-triangle plots where the y axis is actually relevant for the upper triangle and not necessarily 0 to 1?
There was a problem hiding this comment.
the maximum consistent across a triangle plot (keeping in mind that in most cases there won't be a colorbar to go alongside the plot).
Oh I misunderstood the vmax=1 idea. I see the idea, but I think it might be difficult if some PDFs are much narrower than others. Say you have N(sigma=0.1) and N(sigma=1) [not too uncommon], then the 2nd PDF would peak at most at 10% of the height of the plot and be very hard to read.
Thanks Lukas for making the PR!
There was a problem hiding this comment.
Thinking about it, I don't know anymore why I suggested that. Normalizing to maximum and setting vmax=1 is no different to setting vmax=pdf.max() in all plots, so no need for that.
But I do think that vmax=pdf.max() is the default anyhow, so we don't need to code it in there and by not putting it in explicitly we allow for vmax to be overwritten if desired, right? I've removed it in babcf52.
There was a problem hiding this comment.
@Stefan-Heimersheim, could you confirm this achieves what you want, and if so approve the review so @lukashergt can squash and merge
…verwriting the vmax kwarg
Codecov Report
@@ Coverage Diff @@
## master #157 +/- ##
=======================================
Coverage 94.98% 94.98%
=======================================
Files 16 16
Lines 1614 1615 +1
=======================================
+ Hits 1533 1534 +1
Misses 81 81
Continue to review full report at Codecov.
|
This addresses #156 which pointed out that
vminin 2d histograms needs to be set to zero in order for uniform distributions to appear uniform.Fixes #156
Checklist:
flake8 anesthetic tests)pydocstyle --convention=numpy anesthetic)python -m pytest)