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

undeprecate colorCalculator and fix the fishy code #1493

Closed
gordonwoodhull opened this issue Oct 19, 2018 · 5 comments
Closed

undeprecate colorCalculator and fix the fishy code #1493

gordonwoodhull opened this issue Oct 19, 2018 · 5 comments

Comments

@gordonwoodhull
Copy link
Contributor

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
Copy link
Contributor Author

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
Copy link
Contributor

tttp commented Oct 24, 2018

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
Copy link
Contributor Author

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
Copy link
Contributor Author

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
Copy link
Contributor Author

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

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

No branches or pull requests

2 participants