Skip to content

sort bins #334

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

Closed
wants to merge 7 commits into from
Closed

sort bins #334

wants to merge 7 commits into from

Conversation

Fil
Copy link
Contributor

@Fil Fil commented Apr 20, 2021

The proposed syntax specifies sort in the reduce/outputs object, enabling sorting on a different reducer.

      Plot.dot(data, Plot.bin({
        r: "count",
        sort: "count",
        reverse: true
      }, {
        x: "body_mass_g",
        y: "culmen_length_mm",
        stroke: "species",
        fill: "species"
      }))

closes #297

@Fil Fil requested a review from mbostock April 20, 2021 20:27
@enjalot
Copy link

enjalot commented Apr 28, 2021

I prefer the alternative syntax where we sort by the output of the bin/group (most of the time I'm going to want to see biggest / smallest aggregates).
It would also be nice if the option was "sort" to match the Mark syntax

@Fil
Copy link
Contributor Author

Fil commented Apr 28, 2021

Thanks for the feedback. I've pushed the requested change (sort is now a reducer, in the outputs object).

Re: sorting the bars by height, the issue has been discussed in #106 #108, and finally set aside in favor of using an explicit d3.groupSort. [EDIT:] now discussed as part of #442.

@Fil Fil marked this pull request as ready for review April 28, 2021 19:42
@Fil
Copy link
Contributor Author

Fil commented May 1, 2021

A complementary approach would be to automatically sort dots by r (if r is a channel), and maybe lineX (on y) and lineY (on x)?

@mbostock
Copy link
Member

This is very similar to what I propose for filter! #495

@mbostock mbostock force-pushed the fil/sort-bins branch 3 times, most recently from f57714f to 7b21cc1 Compare August 11, 2021 02:27
Copy link
Member

@mbostock mbostock left a comment

Choose a reason for hiding this comment

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

Oops, I just realized there’s an issue here. This sort option allows a corresponding sort input so that you can, for example, sort by sum of weights. However, the sort input channel is retained in options and thus is also applied as a basic sort transform prior to the bin and group transform. That seems confusing: either you intended the sort option to be the basic sort transform, or you intended the sort option to be the input to the sort reducer, but not both. We should try to avoid this ambiguity.

Another issue is that sorting is a little different than most reducers: you don’t strictly want to compute an output value; instead you want to define a total order. And hence it’d be nice if you could specify a comparator to the sort output in the same way you can to the basic transform.

I’ll try to work on this tomorrow.

@mbostock
Copy link
Member

mbostock commented Aug 11, 2021

I’m not sure about the comparator issue, but as far as the input channel vs. basic transform ambiguity goes with the sort option, I’m thinking we can resolve that by changing how we interpret the sort option based on the presence or absence of the sort output: if a sort output is specified, then the sort option represents the input channel; but otherwise the sort option is a basic transform that is applied before the bin or group. (And you can always make this explicit using Plot.sort if you like.) This approach could be extended to the filter option in the other PR, too.

@Fil Fil mentioned this pull request Aug 11, 2021
@mbostock
Copy link
Member

I am going to merge this into the filter option PR #495 so we can do both simultaneously.

@mbostock mbostock closed this Aug 11, 2021
@mbostock mbostock deleted the fil/sort-bins branch August 11, 2021 16:22
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Sort bins and groups?
3 participants