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

New Feature: Adding threshold and polarity to horizon.js #48

Open
tildeleb opened this issue Apr 4, 2013 · 7 comments
Open

New Feature: Adding threshold and polarity to horizon.js #48

tildeleb opened this issue Apr 4, 2013 · 7 comments

Comments

@tildeleb
Copy link

tildeleb commented Apr 4, 2013

I am working on an enhancement to horizon.js where there is a settable threshold and polarity for determining which values are plotted above and which are plotted below. The defaults for the current behavior are:

threshhold = 0,
polarity = -1,

but for example :

threshhold = 50,
polarity = 1,

will draw values over 50 as "negative" or plotted on top.

  • Does anyone else think this is useful?
  • Would this change be accepted for a pull request?
@RandomEtc
Copy link
Collaborator

Sounds interesting. Can you post some before/after screenshots? I'm having a hard time imagining it :)

Happy to look at a pull request and will likely merge anything that is: (a) backwards compatible and (b) coded consistently with the rest of the library.

@mbostock
Copy link
Collaborator

mbostock commented Apr 4, 2013

You could do this with metric arithmetic, although might be confusing because the displayed value in the label will still match the chart (and it sounds like you want to decouple the two). Something like metric.subtract(50).multiply(-1).

@tildeleb
Copy link
Author

tildeleb commented Apr 4, 2013

Right. Metric arithmetic will make the chart look like I want but the values displayed won't be what the user wants to see. Consider something like packet latency in units of ms. You want to be able to see when the latency is above a threshold of 50 ms plotted on "top" in red and below 50 ms in green on the "bottom", but you don't want 50 to read as 0.

All this does is allow a selectable "pivot point" for that determines which data are plotted from "above" and "below". The polarity field determines simply which values are considered/plotted "above" and which "below".

The additional lines of code needed to do this are very few and the default behavior is 100% backward compatible. I'l post a patch and example soon so others can get a feel for it. I already have several applications in mind for my own work and customer work.

@mbostock
Copy link
Collaborator

mbostock commented Apr 4, 2013

The other thing I would consider is that you can define the bands of the horizon chart by way of the extent the band colors, so you can implement semantic thresholds without inverting values below that threshold. This approach might be simpler and more intuitive. For example, if you said something like

horizon
    .extent([0, 100])
    .colors(["black", "black", "lightgray", "orange"]);

Then values less than 50 would be gray and values greater than 50 would be orange. (I’m assuming no negative values in this example.)

@tildeleb
Copy link
Author

tildeleb commented Apr 5, 2013

From a UI/UX perspective you maybe be entirely right and thank you for the suggestion. I will experiment with your suggestion. Still, I think the restriction that the data must to pivot around 0 is an artificial one.

One thing I realized during some testing is that this implementation of horizon graphs is that data values of 0 aren't plotted. In my version data at the pivot/threshold isn't plotted which is consistent. This isn't a bug per-se and I am going to open a separate issue for that.

For example in my test case "threshold=80" and "polarity=1" data values of 80 ms aren't plotted. I have the same issue when plotting something like packet loss % where 0 is the common case. Any thought on this?

Do you prefer the name "threshold" or "pivot" for the variable/fuction that determines if values are plotted from above or below?

I almost have a patch ready.

@RandomEtc
Copy link
Collaborator

Never missing an opportunity to bike-shed, I would say threshold. But if you already chose pivot don't change it. Both are good names.

Screenshots would definitely help illustrate this feature (and github supports drag-and-drop image uploading in discussion threads). Or an example with fake data on bl.ocks.org if you have time?

@tildeleb
Copy link
Author

tildeleb commented Apr 6, 2013

I will get something up "soon" for others to look at. I had another idea about how to structure the changes I want to try out first. Sorry progress has been slow but I have some other gigs I need to work on too.

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

3 participants