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

Introducing histogram* bingroup attributes #3845

Merged
merged 11 commits into from
May 21, 2019
Merged

Introducing histogram* bingroup attributes #3845

merged 11 commits into from
May 21, 2019

Conversation

etpinard
Copy link
Contributor

@etpinard etpinard commented May 8, 2019

resolves #3749 (the 1d overlay case) and resolves #3515 (the 2d case).

Users can now group traces such that their autobin settings are compatible (see #3515 (comment) for more info on what compatible means).

Examples:

In brief, this PR:

  • adds attribute bingroup to histogram, histogram2d and histogram2dcontour traces
  • histogram2d and histogram2dcontour traces also get new attributes xbingroup and ybingroup to allow users to group x-bins and y-bins separately.

To do so, I decided to:

  • make all histogram* traces (i.e. 1d and 2d) go through the same crossTraceDefaults method
  • make all histogram* traces use that same calcAllAutoBins routine

@plotly/plotly_js best to review this PR commit-by-commit. Starting by reviewing the jasmine tests added in 359fbbc might make things easier to grasp.

- design to work for histogram AND histogram2d* traces!
- design to work with bingroup!
- fills in fullLayout._histogramBinOpts for all histogram* traces
- replace `binDir` field in histogramBinOpts[i] by `dirs` (an array)
  to handle cases where x and y bins are in same bingroup
@antoinerg
Copy link
Contributor

antoinerg commented May 17, 2019

Thanks for the very nice PR @etpinard ! The tests are very nice 👍

I think I have uncovered a bug (not related to your PR) when reviewing. A few mocks in your test suite do not render! I will leave it up to you to decide whether to fix this here or in another PR. Let me know what you decide to do.

... in histogram tests as a workaround for
    #3881
@etpinard
Copy link
Contributor Author

I think I have uncovered a bug (not related to your PR) when reviewing

Thanks for noticing that! Something is (still) up with single-value histograms under barmode: overlay. Tracked now in #3881. Commit 3776cd2 makes the data/layout used in the new crossTraceDefaults tests render ok.

@antoinerg
Copy link
Contributor

Thanks for noticing that! Something is (still) up with single-value histograms under barmode: overlay. Tracked now in #3881.

Thank you for clarifying the issue I encountered! :)

The rest looks good to me! Nice feature @etpinard 💪

💃

@etpinard etpinard merged commit 08e2d7e into master May 21, 2019
@etpinard etpinard deleted the bingroup-finalist branch May 21, 2019 15:05
etpinard added a commit that referenced this pull request May 29, 2019
- this fixes some shared histogram2d (via bingroup) edge cases
- this fixes a performance regression from #3845
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature something new
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Shared autobin for overlaid histograms Shared / matching auto-binning for histogram2d traces
2 participants