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

Implement error bars using standard deviation on timeline plots #204

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

str4d
Copy link
Contributor

@str4d str4d commented Jun 9, 2016

Recreated from #113 on current master.

@str4d
Copy link
Contributor Author

str4d commented Jun 9, 2016

Two things:

@tobami
Copy link
Owner

tobami commented Jun 9, 2016

Reviewing #203 ...

One thought occurred to me: before trying to re-add a complex feature like error bars, do you think it makes sense to reevaluate now what plotting library we use? jqPlot has been unmaintained for over two years now. flot is pretty similar and still widely used, if a bit simpler (may not have all the features). There are also other alternatives. Any thoughts here?

@str4d
Copy link
Contributor Author

str4d commented Jun 9, 2016

I don't personally have an opinion. I've never used any JavaScript plotting libraries prior to setting codespeed up; since then I have now used jqPlot and d3, but only just enough to get the features I wanted working 😜

@tobami
Copy link
Owner

tobami commented Jun 9, 2016

Ah, it seemed like you had experience with plotting libraries! Let's leave that question for now then.

@tobami
Copy link
Owner

tobami commented Jul 31, 2016

@str4d are you going to rebase this branch onto master?

@str4d
Copy link
Contributor Author

str4d commented Sep 13, 2016

Sorry for the delay, work's been keeping me busy. I'll do this Friday 😄

@tnorth
Copy link

tnorth commented Oct 18, 2016

I'm also interested in this change, therefore a kind ping @str4d :-)

@tnorth
Copy link

tnorth commented Oct 20, 2016

Ok, I merged it quickly, it works for me but since I don't know the internals of codespeed, I can't tell if I broke something.
The merge commits are here: https://github.com/tnorth/codespeed/commits/master

@tobami
Copy link
Owner

tobami commented Oct 20, 2016

Nice, thanks @tnorth! I'll do some "visual" testing of it 😄

@str4d
Copy link
Contributor Author

str4d commented Aug 22, 2017

Sorry for dropping the ball on this! I'll be doing some more codespeed work in the next few weeks, so I'll get this integrated while I do :)

@str4d
Copy link
Contributor Author

str4d commented Aug 22, 2017

Rebased on 0.11.0 (because that's what I've been working from locally), and fortunately there are no merge conflicts with master!

@tobami
Copy link
Owner

tobami commented Aug 25, 2017

Could you rebase on top of master just in case? I merged some big refactors affecting default branches.

jqplot.ohlcRendererWithErrorBars.min.js is the standard OHLC Renderer plugin
from jqPlot 1.0.9 (revision d96a669) with the error-bar addition given here:

https://bitbucket.org/cleonello/jqplot/issues/35/add-error-bar-capability#comment-141580
@str4d
Copy link
Contributor Author

str4d commented Aug 25, 2017

Rebased on master, still appears to work for me!

@tobami
Copy link
Owner

tobami commented Aug 26, 2017

I checked and the error bars are shown, well done! However, the padding has been increased.

Before:
image

After:
image

(this is with testdata.json)

@str4d
Copy link
Contributor Author

str4d commented Aug 26, 2017

That padding change appears to be left over from the original PR #113 (see e.g. #113 (comment)). I can try playing around with the contents, but not being the original author I don't immediately know the cause.

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.

4 participants