Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Histogram can't handle single-bin data #100

Closed
PAK90 opened this issue May 22, 2018 · 13 comments · Fixed by #119
Closed

Histogram can't handle single-bin data #100

PAK90 opened this issue May 22, 2018 · 13 comments · Fixed by #119
Labels

Comments

@PAK90
Copy link

PAK90 commented May 22, 2018

I'm attempting to make a crossfilter-esque dashboard and part of that involves using the histograms. They're great, except when I filter my data to include only one bin's worth of data (e.g. reducing data to one category, apples, out of ['apples', 'oranges', 'bananas']). At this point I get an error: Cannot read property 'bin0' of undefined. I'm assuming it's attempting to calculate bins on the category for which apples is a part of, but comes up with only one bin, hence problems. Is there a simple workaround for this?

@williaster
Copy link
Owner

hey @PAK90, thanks for checking out the histogram package and raising the issue! I'll look into this today and hopefully get a fix in for it, or let you know if I find an easy workaround.

@williaster
Copy link
Owner

@PAK90 can you link to a codesandbox to reproduce this? In my own testing for pre-binned or un-binned categorical data, I do not encounter this error.

@PAK90
Copy link
Author

PAK90 commented May 23, 2018

Here's a sample of the data I'm passing that's giving me trouble: https://gist.github.com/PAK90/dbe47c09ba90a7b3e9ebd118a42270a4. Note all the fields labeled 'manaCost' are identical (11).

And here's my code for the histogram in case I'm doing something silly in it:

<Histogram
          ariaLabel="Mana cost"
          orientation="vertical"
          height={200}
          width={400}
          cumulative={false}
          normalized={false}
          valueAccessor={sortedData => {
            return sortedData.manaCost;
          }}
          binType="numeric"
          renderTooltip={({ event, datum, data, color }) => (
            <div>
              <strong style={{ color }}>{datum.bin0} to {datum.bin1}</strong>
              <div><strong>count </strong>{datum.count}</div>
            </div>
          )}
        >
          <BarSeries
            animated
            rawData={sortedData} // contains the data posted in the gist
          />
          <XAxis label="Mana Cost" />
          <YAxis />
        </Histogram>

@williaster
Copy link
Owner

Ah okay, numeric data not categorical. I can reproduce the error now will try to get a fix out soon.

@williaster
Copy link
Owner

A quick fix for you would be to set binValues to an array of at least 2 numbers on the Histogram component.

@williaster williaster added the bug label Jul 30, 2018
@kaiyoma
Copy link

kaiyoma commented Aug 21, 2018

Any update on this? I just started using this histogram component today and it's really great, except I'm also running into this error scenario.

@williaster
Copy link
Owner

@kaiyoma yes! sorry for the delay, have been swamped at work. Dug in tonight / have a fix for it, will PR soon and should be able to release a new version tomorrow 👍

@williaster
Copy link
Owner

should be fixed in @data-ui/histogram@^0.0.63

@kaiyoma
Copy link

kaiyoma commented Aug 22, 2018

@williaster I don't think this is quite fixed. If the value in the single bin is greater than 1, the x-axis goes in the wrong direction and the upper bound of the bin is smaller than the lower bound.

Sandbox: https://codesandbox.io/s/p5z87y6ko0

The problem becomes much more exaggerated if the value is much larger than 1, because it seems the upper bound of the bin is always 1.

@williaster
Copy link
Owner

oooof, this looks like it comes from d3 histogram's behavior. In the case where there is one value, it's not clear what the size of the bin should be so d3 returns a bin with equal bin0 and bin1. This is problematic to render because a range can't have the same start and end:

image

curious what you think the expected behavior should be. I could increment the upper bin by 1 for this edge case, but it's not ideal

image

@kaiyoma
Copy link

kaiyoma commented Aug 23, 2018

I think "adding one" to make the upper bound is reasonable. Like you said, it's an edge case, where everything falls into a single bin, so I wouldn't lose any sleep over it. 😄 Making an artificially-sized bin is certainly better than rendering the x-axis in the wrong direction.

@williaster
Copy link
Owner

👌 Released this fix in 0.0.64

@kaiyoma
Copy link

kaiyoma commented Aug 23, 2018

Great, the fix works just like I thought it would. Thanks again!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants