Table.hist(bin_column=...) generates wrong x-axis label

[davidwagner]

If you pass the bin_column parameter to Table.hist(), the x-axis is labelled inappropriately. Consider, e.g.,:

t = Table().with_columns(
    'Value', make_array(2, 3, 9),
    'Proportion', make_array(0.25, 0.5, 0.25)
)

t.hist(bin_column='Value', bins=np.arange(1.5, 9.6, 1))

This produces the following histogram:

Notice how the x-axis got a label based on Proportion. Instead, the x-axis should be labelled with Value.

The bug is in prepare_hist_with_bin_column() in Table.hist(). In particular, it generates the x-axis label with the code w.rstrip(' count') where here w is the label on the column of counts (e.g., Proportion in this case). It seems that the code is expecting/assuming that the label will have a particular format (e.g., that it would be Value count in this case). Instead, the x-axis label should be generated from the column label you passed to bin_column. Thus, it looks like w.rstrip(' count') should be changed to bin_column.

@papajohn, @a-adhikari, do you agree?

[adnanhemani]

@davidwagner I do agree about this. I can put up a change quite quickly (I've figured it out, I think) - but do we need the other instructors to agree to the proposed changes?

[davidwagner]

I don't think we need to wait for the other instructors. This shouldn't change anything for existing usage (without bin_column), and for the rare cases with bin_column, it seems clear enough that the current behavior is broken.

[adnanhemani]

On second thought, I'm not so sure this is the wrong behavior. I think the original use case for bin_column was for something like a 2-column table where one column is the bins and the other column being the count of a unit as related to the first column. For example, a table like this:

t = Table().with_columns("bins", np.arange(4), "cars_seen", [6, 4, 5, 9])

So in this case, a call to Table#hist (ex. t.hist(bin_column="bins")) should have the label "cars_seen" on the x-axis and bins drawn at 0, 1, 2, 3. In the example you've shown above, the "cars_seen" column from the example matches to the "Proportion" column and the "bins" column relates to the "Value" column. If I'm correct, then from this example you've identified a use-case that isn't what we expect from the library but a use-case that (I'd think) is something that the library doesn't support (and may not be able to support due to the expense on the simplicity of the library that supporting a feature like this may occur).

[adnanhemani]

However, I'm getting curious about one thing: If the bin_column parameter's column should be showing the lower end of the bins, in the example you've shown, why isn't the 3 bin extending all the way to 9? Are bins of default size 1 the only thing that's supported with this parameter?

[davidwagner]

Oh. You've got a good point -- now that I look at it again, I think I agree with you.

I ran across this when building the textbook; the example is taken from Chapter 14.1 of the textbook (it's in a hidden cell in the notebook). However now that I read your comments I suspect this might be a misuse of bin_column.

I found only two other places where we use bin_column in the course materials:

• Chapter 7.2 of the textbook

• Lab 5

Based on a superficial glance, both seem to be used in a way consistent to how you mention.

So I'm now thinking the correct fix might be to preserve the existing behavior, and change the way we draw the chart in Chapter 14.1 of the textbook. Does this sound right to you?

Perhaps @papajohn will have an opinion.

[papajohn]

Yeah, Chapter 7.2 was the reason `bin_column` was added, and the behavior is as intended.

…

On Thu, Jun 20, 2019 at 7:30 PM davidwagner ***@***.***> wrote: Oh. You've got a good point -- now that I look at it again, I think I agree with you. I ran across this when building the textbook; the example is taken from Chapter 14.1 of the textbook <https://www.inferentialthinking.com/chapters/14/1/Properties_of_the_Mean.html> (it's in a hidden cell in the notebook <https://github.com/data-8/textbook/blob/7a590fa192fbe8bfc1678661a016cd18aba6bab8/content/chapters/14/1/Properties_of_the_Mean.ipynb>). However now that I read your comments I suspect this might be a misuse of bin_column. I found only two other places where we use bin_column in the course materials: - Chapter 7.2 of the textbook <https://www.inferentialthinking.com/chapters/07/2/Visualizing_Numerical_Distributions.html> - Lab 5 <http://datahub.berkeley.edu/hub/user-redirect/git-sync?repo=https://github.com/data-8/materials-sp19&subPath=materials/sp19/lab/lab05/lab05.ipynb> Based on a superficial glance, both seem to be used in a way consistent to how you mention. So I'm now thinking the correct fix might be to preserve the existing behavior, and change the way we draw the chart in Chapter 14.1 of the textbook. Does this sound right to you? Perhaps @papajohn <https://github.com/papajohn> will have an opinion. — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#337?email_source=notifications&email_token=AACOFEIZOFT4QI6BSMP7G43P3O5E3A5CNFSM4FZDSWF2YY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGODYGCZOQ#issuecomment-504114362>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AACOFEPZGU7L53XTYSA6WXDP3O5E3ANCNFSM4FZDSWFQ> .

[adnanhemani]

Sounds good. I'll close this issue and perhaps we can open an issue to rewrite Chapter 14.1?