-
Notifications
You must be signed in to change notification settings - Fork 186
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
expose projection #2038
base: main
Are you sure you want to change the base?
expose projection #2038
Conversation
/** The projection’s name, if specified. */ | ||
name?: string; | ||
/** A function that projects a point coordinates. */ | ||
point: (point: [number, number]) => [x: number, y: number] | undefined; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should this be apply
to match a scale? (And we might want an invert
, too?)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, but I was wondering if apply should be for geometries, not points. However we can handle both types since they're easy to detect (geometries have a .type
property).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Seems directionally good, but I don’t think it maintains the principle of the materialized projection having the same shape (or being usable) as the projection options, like we do for plot.scale()
. Meaning, we want to be able to call plot.projection()
and then pass the resulting object into Plot.plot
as the projection option and get the same behavior. (Or maybe it does work because of the projection.stream
property and I just missed it?)
This is more the direction in which I'd like to go: expose just enough to make it possible to reuse the projection in another chart (we're not there yet), and to support advanced brushing (uwdata/mosaic#336).
(Not solved yet!)
We need:
This somewhat supersedes @jheer's suggestion in #1191 (comment)
closes #1191