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

Refactored heatmap to allow for custom sorting. Defaults to ascending. #869

Merged
merged 2 commits into from
Mar 3, 2015

Conversation

mtraynham
Copy link
Contributor

The rows/cols setter/getters were doing a bit too much. They should only allow the user to specify a custom domain for the specific axis.

Added a rowOrdering and colOrdering setter/getter to specify a custom sort for each axis. This can be null for no sort. Thus if a user supplies their own domain with no sort, it will use the order they provided in their custom domain.

Also, we can reuse the scale instead of recreating it every time a redraw happens.

@gordonwoodhull
Copy link
Contributor

Good work. I agree that it was odd that the accessors were constructing and returning a scale.

This raises the question whether it's possible (or interesting) to have other kinds of scales for a heatmap. I know for a geographic heatmap it's more like a linear scale (floating point with a certain precision). We can wait to see if anyone asks.

@gordonwoodhull
Copy link
Contributor

This supersedes #837.

@gordonwoodhull gordonwoodhull mentioned this pull request Mar 3, 2015
@gordonwoodhull gordonwoodhull merged commit fd524b4 into dc-js:develop Mar 3, 2015
@mtraynham
Copy link
Contributor Author

Interesting thought on the scales. In some cases, depending on the domain of data, most of these charts could have any type of scale on their X & Y (Z?) axes. I'm tempted to extend that issue beyond just heatmaps (even though they are hard-coded to ordinal).

It would be really cool to have a scale resolver routine, which generates a scale based on the type of data fed to it (configurable of course). Allowing most charts to handle any variant of scale type would be really helpful, and likely simplify chart creation.

On second thought, does a bar chart with an Ordinal scale on the Y-axis, become a heat map?

@gordonwoodhull
Copy link
Contributor

Automatically resolving the scales is a great idea, however I see it as out of scope for dc.js. For the most part, dc.js doesn't infer anything and you have to set everything.

I think it's best to keep it that way and build more libraries on top, like dcplot.js, which does lots of defaulting and inference. I plan to return to that library soon.

However, maybe that is something different from what you're talking about. I don't know if it's possible to create an adapter that makes any scale work with any chart, but ideas are welcome.

I think a heatmap is just a scatterplot with ordinal scales and square dots. ;-)

@mtraynham mtraynham deleted the heatmap_sort branch May 16, 2015 17:59
@renoth
Copy link
Contributor

renoth commented May 29, 2015

When will this be available to the public? 2.0.0-beta10 seems to not have this feature.

@mtraynham
Copy link
Contributor Author

@renoth I targeted the PR at the develop branch as I saw it more of a enhancement rather than a bug. Maybe @gordonwoodhull can cherry pick the commits back to master and include it in the next beta release.

@gordonwoodhull
Copy link
Contributor

Since this changes the interface it has to go into a new version.

Please download the develop version. If you need to use a CDN I can see about starting to distribute 2.1.* but I am afraid that this might break a lot of people's work if they are lazy in how they specify their dependencies.

@gordonwoodhull
Copy link
Contributor

Any suggestions welcome about how to simultaneously distribute a develop and a master version.

@mtraynham
Copy link
Contributor Author

@gordonwoodhull Does a change in the interface that is compatible with pre-existing versions still suggest that it always needs to be in develop?

@gordonwoodhull
Copy link
Contributor

? Sometimes I will merge a change to develop if it is risky, but in general if a commit does not change the interface then it can be merged to master. This one changes the interface for the better, but it returns the values unsorted instead of a scale.

@mtraynham
Copy link
Contributor Author

Ahh, right it does. Overlooked that.

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