dc-js / dc.js

Multi-Dimensional charting built to work natively with crossfilter rendered with d3.js
Apache License 2.0
7.43k stars 1.8k forks source link

undeprecate colorCalculator and fix the fishy code #1493

Closed gordonwoodhull closed 5 years ago

gordonwoodhull commented 6 years ago

Although scales and accessors are bright and shiny, they don't cover all use cases.

In particular, it's much easier to special-case a specific value using the colorCalculator, as seen in the venture capital example. I don't know how to create a scale that returns normal values for everything except for one exact value (undefined in this case).

I had two reasons for deprecating this method:

  1. It's assigning to getColor, which is a horrible horrible thing for a setter to do
  2. I thought that it's always better to use a D3 object than to write code

The second reason isn't all that good - D3 is not supposed to be a framework that stops you from writing code! Sometimes writing a little logic is better than the best-abstracted toolkit.

The first reason can be fixed without hurting efficiency too much.

gordonwoodhull commented 6 years ago

The current deprecation leads to questions like this one:

https://stackoverflow.com/questions/52857893/dc-js-geochoroplethchart-upgrade-from-colorcalculator-to-coloraccessor

Not the first time this has come up. For one, the discussion in #1225 is not conclusive. I've also seen somewhere people switching between different scales based on the data, which is also a reasonable thing to do.

tttp commented 6 years ago

Hi,

I had the same kind of problem as you mentioned for the venture capital, what I ended up doing was: set up a domain [ -42 /*value that isn't par of the normal range*/, 0, 100] and color range [color invalid, color for 0, color for 100] and a color accessor

if value == undefined return -42 
return value;

and kind of work, but really clunky IMO.

I do miss the colorAccessor, +1 adding it back ;)

gordonwoodhull commented 6 years ago

Thanks @tttp, it's good to know there is a workaround but yeah what a mess... why force a scale to perform logic, when the language itself is so much better at it?

gordonwoodhull commented 5 years ago

More recent versions of d3-scale include .unknown() and diverging scales, which cover some of the use cases for colorCalculator. However, a function will inevitably be easier in some cases.

I'm fixing this by adding an override function _colorCalculator, and removing the horrible assignment to .getColor, as well as the deprecation warning.

gordonwoodhull commented 5 years ago

Released in 3.0.11; backported to 2.2.2 for all those folks still using d3v3!