Edgewall Software
Modify

Opened 15 years ago

Closed 14 years ago

#426 closed enhancement (fixed)

Replace XML/SWF Charts with an open system

Reported by: dfraser Owned by: dfraser
Priority: major Milestone: 0.6
Component: Trac plugin Version: 0.6b2
Keywords: charts Cc: silk@…, tim@…, felix.schwarz@…, mpotter@…, hodgestar
Operating System:

Description (last modified by dfraser)

XML/SWF Charts:

  • has some layout problems on some machines (which are probably fixable)
  • is not open source/free software (which is probably not fixable)
  • only generates Flash charts (which is a bit icky)

This ticket is to collect the simplest ideas to replace it with something else. Sample options include (please add serious contenders here):

Attachments (7)

bitten-426-open-charting-1.patch (122.0 KB) - added by dfraser 15 years ago.
Initial reimplementation of charting in flot
bitten-charts.png (240.6 KB) - added by dfraser 15 years ago.
Screenshot (note that lint and coverage aren't enabled on these tests)
bitten-426-open-charting-2.patch (123.2 KB) - added by hodgestar 14 years ago.
Updated patched (use either json or simplejson, move legend, remove old charting templates, remove non-functional xaxis ticks).
bitten-xml-charts.png (34.6 KB) - added by hodgestar 14 years ago.
Screenshot of old XML charts for comparison.
bitten-jquery-charts.png (83.3 KB) - added by hodgestar 14 years ago.
Screenshot of new JQuery charts (made using patch 2).
bitten-426-open-charting-3.patch (159.1 KB) - added by dfraser 14 years ago.
Reworked patch to apply to latest trunk
resizable-charts.diff (1.9 KB) - added by hodgestar 14 years ago.
Add config option for chart style attribute (allows chart width and height to be configured).

Download all attachments as: .zip

Change History (48)

comment:1 follow-up: Changed 15 years ago by silk <silk@…>

  • Cc silk@… added

Just a comment, why I think Google Chart API would not be a good solution.

For closed source development, sending even the data for graphs to external service may be unacceptable (politicaly, etc). I know it would be a big problem for us.

comment:2 in reply to: ↑ 1 Changed 15 years ago by dfraser

Replying to silk <silk@…>:

Just a comment, why I think Google Chart API would not be a good solution.

For closed source development, sending even the data for graphs to external service may be unacceptable (politicaly, etc). I know it would be a big problem for us.

Absolutely, I just included it for reference as it was used by other Trac extensions.

comment:3 Changed 15 years ago by cboos

  • Type changed from task to enhancement

Definitely a must. I would suggest a jQuery based solution, like Flot.

comment:4 Changed 15 years ago by mgood

jqPlot is another nice jQuery-based library.

Changed 15 years ago by dfraser

Initial reimplementation of charting in flot

comment:5 Changed 15 years ago by dfraser

The attached patch includes the flot.js and excanvas.js files. It seems to work, but not been seriously tested. There's definitely room for work on visuals, but for me it's already nicer than the flash charts :-)

Changed 15 years ago by dfraser

Screenshot (note that lint and coverage aren't enabled on these tests)

comment:6 Changed 14 years ago by cmlenz

gRaphaël looks pretty darn nice, too.

comment:7 follow-up: Changed 14 years ago by Tim Niemueller <tim@…>

  • Cc tim@… added

I have packaged Bitten for Fedora and FreeBSD and I'm about to submit these packages for inclusion. For Fedora this is not possible with the given chart library, given that it is not open source and therefore not acceptable as per http://fedoraproject.org/wiki/Licensing.

We are not using charting feature in our project, yet. Hence I didn't test the supplied patch. Can someone confirm that the patch is working and a equal replacement?

In that case I'll include the open charting patch in the package and remove the existing stuff. Is there a chance to see this already in 0.6, given that there is a patch available that can do this already?

comment:8 in reply to: ↑ 7 ; follow-up: Changed 14 years ago by anonymous

Replying to Tim Niemueller <tim@…>:

I have packaged Bitten for Fedora and FreeBSD and I'm about to submit these packages for inclusion. For Fedora this is not possible with the given chart library, given that it is not open source and therefore not acceptable as per http://fedoraproject.org/wiki/Licensing.

That's great news...

We are not using charting feature in our project, yet. Hence I didn't test the supplied patch. Can someone confirm that the patch is working and a equal replacement?

The patch is working but is not yet an equal placement - the charts it generates are functional but fairly ugly.

In that case I'll include the open charting patch in the package and remove the existing stuff. Is there a chance to see this already in 0.6, given that there is a patch available that can do this already?

Given this was a first stab and 0.6 is a desparate-release-because-none-has-been-made for ages, it was meant to be deferred... you could bring up a discussion on IRC/the mailing list if you want

comment:9 in reply to: ↑ 8 Changed 14 years ago by dfraser

Replying to anonymous:

Replying to Tim Niemueller <tim@…>:

We are not using charting feature in our project, yet. Hence I didn't test the supplied patch. Can someone confirm that the patch is working and a equal replacement?

The patch is working but is not yet an equal placement - the charts it generates are functional but fairly ugly.

Forgot to login - that comment was from me (dfraser)

comment:10 follow-up: Changed 14 years ago by Tim Niemueller <tim@…>

Are you working on this atm? Is it feature-wise equali with the flash charts despite looking ugly? What has to be done otherwise? Would you say it is an acceptable compromise to use your patch and getting it packaged and included in Fedora? Otherwise that would have to wait for some time...

comment:11 in reply to: ↑ 10 Changed 14 years ago by dfraser

Replying to Tim Niemueller <tim@…>:

Are you working on this atm? Is it feature-wise equali with the flash charts despite looking ugly? What has to be done otherwise? Would you say it is an acceptable compromise to use your patch and getting it packaged and included in Fedora? Otherwise that would have to wait for some time...

I generated the initial patch, and since the plan was for it to be deferred to 0.7 I haven't put any more effort into it. But I am running a production server using it (on Fedora 7, I produce my own rpms) - the plan is for either this patch or an equivalent patch using an alternative graphing system to make it into bitten for the next milestone, so I think per Fedora's policies it would be best to include it (the only alternative being to remove charting). But I think you should test yourself and see since you're the one who's going to submit it.

Changed 14 years ago by hodgestar

Updated patched (use either json or simplejson, move legend, remove old charting templates, remove non-functional xaxis ticks).

Changed 14 years ago by hodgestar

Screenshot of old XML charts for comparison.

Changed 14 years ago by hodgestar

Screenshot of new JQuery charts (made using patch 2).

comment:12 Changed 14 years ago by Felix Schwarz <felix.schwarz@…>

  • Cc felix.schwarz@… added

comment:13 Changed 14 years ago by mpotter@…

  • Cc mpotter@… added

comment:14 Changed 14 years ago by hodgestar

  • Owner set to hodgestar
  • Status changed from new to assigned

comment:15 Changed 14 years ago by hodgestar

  • Cc hodgestar added

Changed 14 years ago by dfraser

Reworked patch to apply to latest trunk

comment:16 Changed 14 years ago by dfraser

The attached patch seems OK to me now - we could potentially do some stylistic adjustments, but as far as I'm concerned, this is at least ready for trunk

comment:17 Changed 14 years ago by dfraser

Committed in r846, but leaving this open for testing...

comment:18 Changed 14 years ago by dfraser

Backported in [846]; as of that point trunk and 0.6.x are identical except for versioning...

comment:19 Changed 14 years ago by anatoly techtonik <techtonik@…>

jQuery sparkline is another library to consider for inline charts http://omnipotent.net/jquery.sparkline/

comment:20 Changed 14 years ago by hodgestar

  • Milestone changed from 0.7 to 0.6
  • Version changed from dev to 0.6b2

comment:21 follow-up: Changed 14 years ago by dfraser

Discussion in mailing list on the added requirement for a JSON parser includes this:

Trac has a JSON encoder on trunk, which handles most simple cases. It is used to serialize e.g. ticket field data for the custom query page. See the bottom of (lines 282 and following):

http://trac.edgewall.org/browser/trunk/trac/util/presentation.py

The plan is to use that as a fallback for the inbuilt JSON library in Python 2.6

comment:22 in reply to: ↑ 21 Changed 14 years ago by dfraser

Replying to dfraser:

Discussion in mailing list on the added requirement for a JSON parser includes this:

Trac has a JSON encoder on trunk, which handles most simple cases. It is used to serialize e.g. ticket field data for the custom query page. See the bottom of (lines 282 and following):

http://trac.edgewall.org/browser/trunk/trac/util/presentation.py

The plan is to use that as a fallback for the inbuilt JSON library in Python 2.6

Unfortunately it's only available in Trac 0.12 onwards. Bleaugh

comment:23 Changed 14 years ago by dfraser

  • Owner changed from hodgestar to dfraser
  • Status changed from assigned to new

Committed a version that handles using trac's json method, or if not Python's, or if not, an internal version in [857], and backported it in [858]

comment:24 Changed 14 years ago by dfraser

  • Status changed from new to assigned

comment:25 Changed 14 years ago by dfraser

  • Description modified (diff)

comment:26 Changed 14 years ago by wbell

Are there any upgrade requirements here? I'm running an instance of Trac 0.11.5 and oddly enough having problems getting to the build pages (/build/config/id) even though the charts look good on the configuration pages. It's not obvious to me why those pages even need json.txt.

Stack trace probably isn't useful, but here's the end:

  File "/usr/lib/pymodules/python2.6/genshi/template/base.py", line 581, in _include
    cls=cls or self.__class__)
  File "/usr/lib/pymodules/python2.6/genshi/template/loader.py", line 227, in load
    filename, encoding=encoding)
  File "/usr/lib/pymodules/python2.6/genshi/template/loader.py", line 265, in _instantiate
    allow_exec=self.allow_exec)
  File "/usr/lib/pymodules/python2.6/genshi/template/base.py", line 379, in __init__
    raise TemplateSyntaxError(e.msg, self.filepath, e.lineno, e.offset)
TemplateSyntaxError: not well-formed (invalid token): line 1, column 0 (/usr/local/lib/python2.6/dist-packages/Bitten-0.7dev-py2.6.egg/bitten/templates/json.txt, line 1)

Running Bitten [857].

comment:27 follow-up: Changed 14 years ago by osimons

I've seen that bug too, but haven't really looked much at the internals of this change. I think perhaps it is related to the Lint reports - at least that is my hunch. I don't get the Lint graph on the config front page, and a build with Lint data won't render throwing the Exception in comment:26.

Walter, do you have Lint data in the builds you can't render?

David, that json.txt template do feel very 'hackish'... Why is is made like that?

comment:28 Changed 14 years ago by dfraser

My bad. When I was fixing the tests I blindly replaced something to use json.txt that was actually the test results renderer. Fix coming now...

json.txt is very hackish and was made like that because the chart summarizer API demands a template - we can look at changing that...

comment:29 follow-up: Changed 14 years ago by dfraser

wbell: Fix committed in [861] and backported in [862]

comment:30 in reply to: ↑ 27 ; follow-up: Changed 14 years ago by dfraser

Replying to osimons:

I've seen that bug too, but haven't really looked much at the internals of this change. I think perhaps it is related to the Lint reports - at least that is my hunch. I don't get the Lint graph on the config front page, and a build with Lint data won't render throwing the Exception in comment:26.

Can you look at the build/$buildname/chart/lint URL directly and see what that returns? I think this is a separate issue

comment:31 in reply to: ↑ 29 Changed 14 years ago by anonymous

Replying to dfraser:

wbell: Fix committed in [861] and backported in [862]

Looks good-- build pages are back.

comment:32 in reply to: ↑ 30 ; follow-up: Changed 14 years ago by osimons

Replying to dfraser:

Can you look at the build/$buildname/chart/lint URL directly and see what that returns? I think this is a separate issue

Sorry, false alarm. A result of a flaky development setup that seems to have rebuilt my "Lint" config using a recipe without working lint. Fixed that, rebuilt, and graphs display. Please ignore :-)

Also verified the latest fix as well, as all builds are displayed as expected.

Nice work! I'll now have a look at the json.txt template thingy.

comment:33 in reply to: ↑ 32 Changed 14 years ago by osimons

Replying to osimons:

I'll now have a look at the json.txt template thingy.

Reusing the Trac infrastructure, this seems to be the right way. It may look a bit odd, but the only other option is to write the request ourselves and raise RequestDone. Which we already do in bitten.master.BuildMaster._send_response() of course. We could refactor that into a shared utility function (along with the error-helper), and do away the need for a template and the rendering overhead.

Changed 14 years ago by hodgestar

Add config option for chart style attribute (allows chart width and height to be configured).

comment:34 follow-up: Changed 14 years ago by hodgestar

The new charts are a bit small for me but I realise some configuration pages are pressed for space so I add a configuration option for setting the chart style (and thus the width and height). I thought about adding something for just setting the width and height but setting the style seemed cleaner and more flexible (although I'm not completely sold on this).

Thoughts?

comment:35 Changed 14 years ago by anatoly techtonik <techtonik@…>

My thought is that configuration file is big enough even without styles, and any style modifications should belong to template customization level. The recipe can go into http://bitten.edgewall.org/wiki/AddingCharts and Adding Charts probably merged into documentation.

comment:36 Changed 14 years ago by anonymous

Overriding the report template wouldn't help here because the relevant style is part of the build config template. For the record, this would be the 9th Bitten configuration option and the user is free to ignore it.

Aside: The advice on the Adding Charts page is pretty horrendous. It would be better to create a tiny Trac plugin that registers its own entry points than to hack up Bitten for this purpose.

comment:37 follow-up: Changed 14 years ago by anatoly techtonik <techtonik@…>

BTW, where is the official documentation on enabling charts in 0.6b2? In my installation they doesn't render.

comment:38 in reply to: ↑ 37 Changed 14 years ago by dfraser

Replying to anatoly techtonik <techtonik@…>:

BTW, where is the official documentation on enabling charts in 0.6b2? In my installation they doesn't render.

There isn't any AFAIK. Are you referring to 0.6b2 or current svn? (They're vastly different) The charts should be automatically created if you have output that can be rendered by the appropriate chart creators. If they aren't (and I'm aware that there are issues with this sometimes), you can look at the generated chart data at http://$tracbaseurl/build/$buildname/chart/$chartcategory (where $chartcategory is test, lint, or coverage) - if that returns an error, or there is no data, then the chart will currently disappear

comment:39 Changed 14 years ago by anatoly techtonik <techtonik@…>

Thanks for clarification. Now I see that test results charts are not generated, because the test results were not provided. I need to see which XML format is more simple for implementation.

comment:40 in reply to: ↑ 34 Changed 14 years ago by hodgestar

I committed the resizable charts patch in r892 and backported it to 0.6.x in r893.

comment:41 Changed 14 years ago by hodgestar

  • Resolution set to fixed
  • Status changed from assigned to closed

I think we can declare this ticket closed -- any problems that arise now deserve their own tickets. Welcome to a completely open and free Bitten. :D

Add Comment

Modify Ticket

Change Properties
Set your email in Preferences
Action
as closed The owner will remain dfraser.
The resolution will be deleted. Next status will be 'reopened'.
Author


E-mail address and user name can be saved in the Preferences.

 
Note: See TracTickets for help on using tickets.