d3 / d3-shape

Graphical primitives for visualization, such as lines and areas.
https://d3js.org/d3-shape
ISC License
2.47k stars 307 forks source link

Removed implicit conversion of points for curve functions #87

Closed Root-Core closed 7 years ago

Root-Core commented 7 years ago

Hello,

it might be required to deliver meta information with your value, if you are using an custom curve. The conversion is not necessary neither, as the d3 provided curve functions already do the trick.

So we are limiting the point function of an curve to number types only. If you are using an self defined curve class, you will take care of the given data, if not you will use the build in ones that will do.

It would be nice to hear your opinion. :) Regards!

mbostock commented 7 years ago

This does not feel like an appropriate change for me. For one, you’d also need to change the behavior when line.x and line.y are set to a constant, and change d3.area, d3.radialArea, and d3.radialLine. But it feels like you are overloading the abstraction if x and y are anything but numbers. It’s not clear from your description what exactly you are trying to accomplish, but my guess is there’s another way to do it that would be cleaner, and in the worst case, you could just fork the d3.line implementation to allow your desired alternative behavior (since whatever new curve you are implementing would function quite differently from the existing curves).

Root-Core commented 7 years ago

Thank you for your answer. The curve, that I have implemented has three Y-Value per X-Value. It has an min, max and the last value, where the last value gets drawn until a new value is set. This has to do with the resolution, if there is more than one value per pixel.

I am using something like this to accomplish a valid y component:

let value = new Number(this._yAxis.scale(d.last));
value.min = this._yAxis.scale(d.min);
value.max = this._yAxis.scale(d.max);

This works with the build in curves and my own, that can handle the additional properties. As the build in curves do an implicit type conversation, we should not need to do so twice and are more flexible for more elaborated approaches. There might be an other way around it, but it looked straight forward.