Switch to flotr2 library from flot for graphing

rufuspollock commented 11 years ago

http://humblesoftware.com/flotr2/

lievenjanssen commented 11 years ago

I'm working on it. Got the graphs working, now changing the tooltips code.

rufuspollock commented 11 years ago

@lievenjanssen that is amazing. How are you feeling that flotr2 stacks up against flot btw?

lievenjanssen commented 11 years ago

Flotr2 introduces some extra functionality out of the box (e.g. tooltip)

By moving from jquery-flot to flotr2 there is no hard dependency on jquery anymore which is a good thing for such a large lib.

Unfortunately there was still a bug in the flotr2 tooltip code (mixed up east-west) but I fixed it and did a pull request on the flotr2 github. Can we temporarily add my fixed version in the vendor folder until the pull request gets merged in the flotr2 master branch?

rufuspollock commented 11 years ago

@lievenjanssen I've just checked out the new flotr2 code in detail and it looks good. I have a few comments:

When putting stuff in the vendor folder can we specify the version (or failing that the date you got the code). E.g. we should have folder structure flotr2/{version-or-date-in-iso-format}/flotr2.js. I appreciate you have added your patched version so this is more complex :-) (i guess you'd do something like flotr2/2012-07-16/flotr2.js but with a note in the commit log.
Bar / column charts do not seem to use the name of the field if that field is a label rather than a number. To see this try out the demo
You hadn't patched the vendor requirements for the demos and tutorials (patch the _includes/recline-deps.html file) which meant they weren't working. A useful thing to note for any future patches. Also no update to the tutorial ... (this could wait on patching the dist). -- I've fixed this ...
If you use the date column you only seem to get the month
If you use columns or bars with dates you get JS number representation of the date rather than the date.

lievenjanssen commented 11 years ago

couple of those issues were already in the original code as well. I'm doing a more thorough test of the graphs (my local vesion has some of those bugs fixed already) and will probably have a new pull request ready by the end of the weekend.

I will also incorporate all your comments.

rufuspollock commented 11 years ago

amazing!

lievenjanssen commented 11 years ago

During bug fixing I came across some functional considerations we have to decide on, these is the way I suggest to implement them:

Group Column (x-axis) is date:

show linear time axis for lines & points, lines and points graph types
show actual dates for bars and columns graph types (possibly sorted ascending by date)

Group Column (x-axis) is string:

group the data by that column and summarize the Value Columns (possibly sorted ascending by group column)

e.g.

Group Columnn, Value Column UK , 10 DE , 5 UK , 5

Should become (grouped + sorted): DE , 5 UK , 15

The aggregation function could also be chosen for the series, e.g. min, max, sum, avg, count.

rufuspollock commented 11 years ago

@lievenjanssen - sorry for delay in responding (I could swear I submitted a comment on Monday but it doesn't seem to be here!)

I think this sounds good except for the last part on how we deal with repeated string values. In this case where x-axis (or y-axis for horizontal bar) is a string value I'd just use the index of the record to set its location (index will be 0,1,2,3 ...) and use the string value to set the label. This way we can cope with repetitions without any problem. This was the approach being used in the old bar charts (see https://github.com/okfn/recline/blob/a58f5e5bb06ed62cdf7d9f881fd8c389bad15b37/src/view.graph.js#L116 and https://github.com/okfn/recline/blob/a58f5e5bb06ed62cdf7d9f881fd8c389bad15b37/src/view.graph.js#L174)

lievenjanssen commented 11 years ago

I know this was the case before but it seems odd to have repeated string values as you can't make sense out of them when displayed on a graph.

Of course grouping and aggregation could be handled by the backend as well.

I will leave the implementation of the repeated string values as it is (index to set location) and do a pull request later this week.

rufuspollock commented 11 years ago

@lievenjanssen sounds great (while I agree re repeated values the primary use case will be where you don't have that -- e.g. you have a nice set of distinct values so all will work well!).

rufuspollock commented 11 years ago

FIXED. May be some minor tweaks remaining but conversion has been done and appears to be operating nicely.

datopian / datahub

Switch to flotr2 library from flot for graphing #170