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

Add band/point invertExtent. #64

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open

Add band/point invertExtent. #64

wants to merge 1 commit into from

Conversation

jheer
Copy link
Contributor

@jheer jheer commented Jun 8, 2016

Adds invertExtent method for band and point scales, which can be useful for brushing and linking over ordinal domains. Separated from earlier PR #60.

@mbostock
Copy link
Member

mbostock commented Jun 8, 2016

I’d like the documentation to be clearer. Given this representation of bands:

Some questions: Is a band included if [r0, r1] has a non-empty intersection with the band, or does [r0, r1] have to completely cover the band? And are r0 and r1 both treated as inclusive bounds? Is a point scale treated as zero-width bands? If [r0, r1] falls within the padding between bands, does it return an empty array? It’d be helpful to have tests which explicitly state the expected outcome for these cases.

@jheer
Copy link
Contributor Author

jheer commented Jun 8, 2016

  1. Non-empty intersection with a band is sufficient for inclusion; r0 and r1 are treated as inclusive bounds. Related cases are included in the submitted tests (e.g., band-test.js#L309).
  2. A point scale is treated as having zero-width bands. Zero bandwidth scales are not explicitly included in the tests, but band boundary tests are.
  3. If the queried range falls entirely within padding, invertExtent returns undefined. This case is included in the submitted tests (e.g., band-test.js#L340, though more detailed comments might be helpful).

@williaster
Copy link

williaster commented Aug 1, 2017

was there any update on merging this or whether there would be .invert() support for band/point scales? would ❤️ this functionality.

@jheer
Copy link
Contributor Author

jheer commented Aug 1, 2017

@williaster If it is useful to you, we have an updated version of band/point scales included in the vega-scale repo. It includes an invert function in addition to modifications to improve layout consistency. We'd be happy to have those merged here if there is interest.

@mbostock
Copy link
Member

mbostock commented Aug 2, 2019

I’ve implemented a demo of ordinal brushing here:

https://observablehq.com/@d3/ordinal-brushing

I used bisectRight as in this PR. One point of awkwardness is x.domain().map(x) to access the underlying ordinal scale’s range. My preference for this type of interaction would be to only select ordinal values based on the middle of the band (in effect, treating band and point scales the same way). I wonder whether the proposed band.invertExtent here could be flexible enough to support different intersection logic, or if some other API is desired?

@mbostock
Copy link
Member

mbostock commented Aug 2, 2019

After a little more thought, I switched my demo to use a point scale. I think it makes sense for the point scale to consider something selected only if the point is inside the interval, while the band scale would consider something selected if the band intersects the interval, as implemented here. So, I’m down with this approach—I just need to think through the API a little more.

@AbreezaSaleem
Copy link

Any update on this? It'll be really useful to have an invert function for band scales.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Development

Successfully merging this pull request may close these issues.

5 participants