Skip to content

Conversation

@mbostock
Copy link
Member

@mbostock mbostock commented Jun 9, 2022

Fixes one part of #928.

I was hoping there’d be a way to do this for the hexbin transform too, but I couldn’t wrap my head around it. The problem is that the initial options (passed to the Mark constructor) need to declare the channels prior to the initializer running, not the channels after the initializer runs; if we blindly declare the output channels in options, then we’ll overwrite the input channels to the hexbin transform. The dodge transform doesn’t have this problem because it doesn’t have a circular channel dependency (it consumes x and outputs y, or consumes y and outputs x; whereas the hexbin transform consumes x and y and outputs x and y, at a minimum).

Anyway, this seems like an improvement. 🤷

@mbostock mbostock requested a review from Fil June 9, 2022 16:06
Fil added a commit that referenced this pull request Jun 9, 2022
A complement to #930

the X and Y channels should be optional in all the voronoi marks—since d3-delaunay can handle collinear points (and it's not just for the sake of it: the resulting voronoi can be useful, as illustrated in the test case).
@Fil Fil mentioned this pull request Jun 9, 2022
Copy link
Contributor

@Fil Fil left a comment

Choose a reason for hiding this comment

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

Yes, we need this in general.

#931 also addresses this issue in (narrower) case of Delaunay and Voronoi diagrams, but it's not an alternative (we need both).

mbostock added a commit that referenced this pull request Jun 10, 2022
* delaunay X and Y are optional

A complement to #930

the X and Y channels should be optional in all the voronoi marks—since d3-delaunay can handle collinear points (and it's not just for the sake of it: the resulting voronoi can be useful, as illustrated in the test case).

* adopt applyFrameAnchor

* one fewer closure

Co-authored-by: Mike Bostock <[email protected]>
@mbostock mbostock merged commit c6ad4bb into main Jun 10, 2022
@mbostock mbostock deleted the mbostock/initializers-declare-channels branch June 10, 2022 16:17
mbostock added a commit that referenced this pull request Jun 10, 2022
@mbostock
Copy link
Member Author

I am reverting this. #931 feels like a better fix, and this was causing issues in #929.

mbostock added a commit that referenced this pull request Jun 10, 2022
@mbostock mbostock mentioned this pull request Jun 13, 2022
backend-devloper pushed a commit to backend-devloper/plot that referenced this pull request Nov 24, 2023
* delaunay X and Y are optional

A complement to observablehq/plot#930

the X and Y channels should be optional in all the voronoi marks—since d3-delaunay can handle collinear points (and it's not just for the sake of it: the resulting voronoi can be useful, as illustrated in the test case).

* adopt applyFrameAnchor

* one fewer closure

Co-authored-by: Mike Bostock <[email protected]>
tigrevol8888 added a commit to tigrevol8888/plot that referenced this pull request Jul 5, 2024
* delaunay X and Y are optional

A complement to observablehq/plot#930

the X and Y channels should be optional in all the voronoi marks—since d3-delaunay can handle collinear points (and it's not just for the sake of it: the resulting voronoi can be useful, as illustrated in the test case).

* adopt applyFrameAnchor

* one fewer closure

Co-authored-by: Mike Bostock <[email protected]>
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.

3 participants