Implemented fix to issue 196 on github for log=True and histtype='step' #1029

Closed
wants to merge 2 commits into
from

Conversation

Projects
None yet
5 participants
Contributor

keflavich commented Jul 20, 2012

A solution was proposed in Issue #196. This pull request implements that proposed solution.

lib/matplotlib/axes.py
@@ -7861,7 +7861,8 @@ def hist(self, x, bins=10, range=None, normed=False, weights=None,
x[0::2], x[1::2] = bins, bins
- minimum = min(bins)
+ ndata = np.array(n)
+ minimum = (ndata[ndata>0].min())*0.1
@WeatherGod

WeatherGod Jul 20, 2012

Member

wouldn't we only want to do this for 'log'? Also, since bins is a list and the operation is just a simple filter, let's go with that route.

Add a comment for the rationale for the filtering and also for the 0.1 multiplication.

fixed the minimum specification in ax.hist as per @WeatherGod's comment;
only want to set min this way if log is specified.  Also, added case for
normed vs not
Contributor

keflavich commented Jul 20, 2012

@WeatherGod - I think you're right, you only want to do it this way for log. You also want different minima for normed vs not.

Member

WeatherGod commented Jul 21, 2012

Why 0.1? What is special about that number versus any other number? Could it accidentially be related to the fact that the default number of bins is 10 (I think)? If so, then we can't rely on that.

Contributor

keflavich commented Jul 21, 2012

No, 0.1 corresponds to one tick label unit for log plots assuming log base
10.
On Jul 20, 2012 6:26 PM, "Benjamin Root" <
reply@reply.github.com>
wrote:

Why 0.1? What is special about that number versus any other number?
Could it accidentially be related to the fact that the default number of
bins is 10 (I think)? If so, then we can't rely on that.


Reply to this email directly or view it on GitHub:
#1029 (comment)

Member

WeatherGod commented Jul 21, 2012

ok, so probably the only thing left is to add a test for this. If you need help with that, just let me know.

Owner

mdboom commented Jul 30, 2012

It should probably use the log's actual base -- it's 10 by default, but it could be anything.

I think this should be np.min(n). It was wrong in the previous version of code. We want minimum to represent the minimum count value not minimum interval. This screwed up the y axis range later on. #1066

I think there is no separate case for normed/non normed data because we can weight the histogram with negative/factional value. For example:

np.random.seed(0)
ndata = 1000
d= randn(1000)*500
w= randn(ndata)
plt.hist(d,bins=40, weights=w, histtype='stepfilled');
Member

dmcdougall commented Jan 18, 2013

This should go in v1.2.x, it's a fix. I'm milestoning it as such.

@keflavich Are you able to add a test for this? If not I can fork your branch and move this forward.

@ghost ghost assigned dmcdougall Jan 18, 2013

Contributor

keflavich commented Jan 18, 2013

Best if you go ahead; I'll not have a chance for at least a week

Member

dmcdougall commented Jan 27, 2013

Resolved in #1684.

@dmcdougall dmcdougall closed this Jan 27, 2013

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