Skip to content

Date bin shift #1201

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.

Sign up for GitHub

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

Merged
merged 6 commits into from
Nov 28, 2016
Merged

Date bin shift #1201

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
f6b902c
fixes #1151 - auto-shift bins for date histograms similar to numeric
alexcjohnson Nov 26, 2016
1b9b122
refactor numeric bin shift for consistency, and lint
alexcjohnson Nov 26, 2016
11f6830
prune TODOs
alexcjohnson Nov 26, 2016
8628661
robustify histogram_test
alexcjohnson Nov 26, 2016
dcd4be1
default binning in date_histogram mock changed a little
alexcjohnson Nov 26, 2016
0050c1e
clearer docs on date histogram autobins
alexcjohnson Nov 28, 2016
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 0 additions & 1 deletion src/plots/cartesian/axes.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -507,7 +507,6 @@ axes.autoBin = function(data, ax, nbins, is2d) {
var distinctData = Lib.distinctVals(data),
msexp = Math.pow(10, Math.floor(
Math.log(distinctData.minDiff) / Math.LN10)),
// TODO: there are some date cases where this will fail...
minSize = msexp * Lib.roundUp(
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

removed because I have no idea anymore what this was about. We'll wait for a bug report...

distinctData.minDiff / msexp, [0.9, 1.9, 4.9, 9.9], true);
size0 = Math.max(minSize, 2 * Lib.stdev(data) /
Expand Down
3 changes: 1 addition & 2 deletions test/jasmine/tests/histogram2d_test.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -91,8 +91,7 @@ describe('Test histogram2d', function() {

// TODO: even though the binning is done on non-uniform bins,
// the display makes them linear (using only y0 and dy)
// when we sort out https://github.com/plotly/plotly.js/issues/1151
// lets also make it display the bins with nonuniform size
// Can we also make it display the bins with nonuniform size?
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Removed from the scope of this PR because it turned out to be more of a pain than I thought, and it's a really minor effect visually.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ref #360

expect(out.y0).toBe('1970-01-01 03:00');
expect(out.dy).toBe(365.25 * oneDay);

Expand Down
2 changes: 1 addition & 1 deletion test/jasmine/tests/histogram_test.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -156,7 +156,7 @@ describe('Test histogram', function() {
nbinsx: 4
});

// Note: this gives half-day gaps between all but the first two
// TODO: this gives half-day gaps between all but the first two
// bars. Now that we have explicit per-bar positioning, perhaps
// we should fill the space, rather than insisting on equal-width
// bars?
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This effect is a little more important than getting the exact right bin sizes in the 2D case, because here it leads to visible gaps between many of the bars. Unfortunately it's also quite a bit more effort to solve when you consider stacked or grouped traces. Worth making an issue for it?

Expand Down