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

Alternate approach to Ordinal Brushing #1809

Merged
merged 2 commits into from
Feb 15, 2021
Merged

Conversation

kum-deepak
Copy link
Collaborator

This approach makes a key assumption that brushing makes logical sense only if the underlying data has a defined order.

In this approach:

  • Approach to mapping string <-> int is similar (converted as a class for simpler syntax).
  • Dimension key is an integer - derived based on the mapping.
  • Where needed the integer key is mapped back to string - for example, the title, axis label, etc.
  • There is no need for a custom filter handler.
  • The previous implementation assumes that each row has unique keys. This may not be the case with most data sets. So, I have updated that part of the code to work with rows where keys may repeat.

The brushing and re-sorting, in my opinion, do not go well together. For example, if there is a selection and the data get re-sorted (for example because of activities on the other charts), the selection will now include different items, which may not be intended. In my opinion, unless there is a natural ordering in the data, range brushing does not make logical sense.

@kum-deepak
Copy link
Collaborator Author

kum-deepak commented Jan 23, 2021

In dc-v5, this approach simplifies http://127.0.0.1:8888/web/examples/focus-ordinal-bar.html significantly, with hardly any code that relies on the internal workings of dc. Please see commit bf81f80.

@gordonwoodhull
Copy link
Contributor

I agree this is a lot simpler and easier to understand, and also that it is a mess if there is an active selection when the domain gets rearranged. It's great not to have to use a filterHandler.

However, mapping the keys only at the start also precludes the use of remove_empty_bins() and streaming data.

Generally dc.js charts do try to adapt when the domain changes. That's why there was a fake group in the original example - it updated the mapping each time data was pulled. It is really messy to combine mapping and fake group, for sure, but that's the reason it was that way.

I think you've done three simplifications here:

  1. using the integers instead of ordinals throughout
  2. separating logic into a Mapper
  3. only read the set of keys once when the chart is first drawn

I think 1 and 2 are real improvements, but 3 may not work in all situations. So I'm merging this but preserving the old example under the name brush-ordinal-dynamic.

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.

2 participants