Merge into trunk : label-formatters : Code : Cairo Plot

Status:	Needs review
Proposed branch:	lp://qastaging/~mrooney/cairoplot/label-formatters
Merge into:	lp://qastaging/cairoplot
Diff against target:	303 lines (+94/-34) 4 files modified trunk/cairoplot.py (+48/-27) trunk/seriestests.py (+21/-7) trunk/testscripts/compare.sh (+22/-0) trunk/testscripts/timeit.sh (+3/-0)
To merge this branch:	bzr merge lp://qastaging/~mrooney/cairoplot/label-formatters
Related bugs:	Link a bug report

Reviewer	Review Type	Date Requested	Status
Rodrigo Moreira Araújo		2010-07-17	Pending
Review via email: mp+30192@code.qastaging.launchpad.net

Description of the change

Add value/label formatters for displaying axis/values as any kind of string such as date, currency, et cetera.

Revision history for this message

Rodrigo Moreira Araújo (alf-rodrigo) wrote on 2010-07-17:

#

Download full text (13.9 KiB)

Hello there,

As I wasn't too busy, i just reviewed the code. The only thing I didn't get
was the use of this compare.sh script. Why would you like to compare png
files?

Great code btw, merging it soon.

Cheers,

Rodrigo Araujo

2010/7/17 Michael Rooney <email address hidden>

> Michael Rooney has proposed merging lp:~mrooney/cairoplot/label-formatters
> into lp:cairoplot.
>
> Requested reviews:
> Rodrigo Moreira Araújo (alf-rodrigo)
>
>
> Add value/label formatters for displaying axis/values as any kind of string
> such as date, currency, et cetera.
> --
> https://code.launchpad.net/~mrooney/cairoplot/label-formatters/+merge/30192
> You are requested to review the proposed merge of
> lp:~mrooney/cairoplot/label-formatters into lp:cairoplot.
>
> === modified file 'trunk/cairoplot.py'
> --- trunk/cairoplot.py 2009-07-09 21:57:24 +0000
> +++ trunk/cairoplot.py 2010-07-17 21:38:38 +0000
> @@ -1,4 +1,4 @@
> -#!/usr/bin/env python
> +#!/usr/bin/env python
> # -*- coding: utf-8 -*-
>
> # CairoPlot.py
> @@ -288,6 +288,8 @@
> series_legend = False,
> x_labels = None,
> y_labels = None,
> + x_formatter = None,
> + y_formatter = None,
> x_bounds = None,
> y_bounds = None,
> z_bounds = None,
> @@ -303,6 +305,9 @@
> self.titles = {}
> self.titles[HORZ] = x_title
> self.titles[VERT] = y_title
> + self.label_formatters = {}
> + self.label_formatters[HORZ] = x_formatter
> + self.label_formatters[VERT] = y_formatter
> self.max_value = {}
> self.axis = axis
> self.discrete = discrete
> @@ -396,19 +401,17 @@
> self.errors[VERT] = [errory]
>
> def calc_labels(self):
> - if not self.labels[HORZ]:
> - amplitude = self.bounds[HORZ][1] - self.bounds[HORZ][0]
> - if amplitude % 10: #if horizontal labels need floating points
> - self.labels[HORZ] = ["%.2lf" % (float(self.bounds[HORZ][0]
> + (amplitude * i / 10.0))) for i in range(11) ]
> - else:
> - self.labels[HORZ] = ["%d" % (int(self.bounds[HORZ][0] +
> (amplitude * i / 10.0))) for i in range(11) ]
> - if not self.labels[VERT]:
> - amplitude = self.bounds[VERT][1] - self.bounds[VERT][0]
> - if amplitude % 10: #if vertical labels need floating points
> - self.labels[VERT] = ["%.2lf" % (float(self.bounds[VERT][0]
> + (amplitude * i / 10.0))) for i in range(11) ]
> - else:
> - self.labels[VERT] = ["%d" % (int(self.bounds[VERT][0] +
> (amplitude * i / 10.0))) for i in range(11) ]
> -
> + for key in (HORZ, VERT):
> + if not self.labels[key]:
> + amplitude = self.bounds[key][1] - self.bounds[key][0]
> + labels = (self.bounds[key][0] + (amplitude * i / 10.0) for
> i in range(11))
> + if self.label_formatters[key]:
> + self.labels[key] = [self.label_formatters[key](label)
> for label in labels]
> + elif amplitude % 10: #if horizontal labels need floating
> points
> + ...

Hello there,

As I wasn't too busy, i just reviewed the code. The only thing I didn't get
was the use of this compare.sh script. Why would you like to compare png
files?

Great code btw, merging it soon.

Cheers,

Rodrigo Araujo

2010/7/17 Michael Rooney <mrooney@ubuntu.com>

> Michael Rooney has proposed merging lp:~mrooney/cairoplot/label-formatters
> into lp:cairoplot.
>
> Requested reviews:
>  Rodrigo Moreira Araújo (alf-rodrigo)
>
>
> Add value/label formatters for displaying axis/values as any kind of string
> such as date, currency, et cetera.
> --
> https://code.launchpad.net/~mrooney/cairoplot/label-formatters/+merge/30192
> You are requested to review the proposed merge of
> lp:~mrooney/cairoplot/label-formatters into lp:cairoplot.
>
> === modified file 'trunk/cairoplot.py'
> --- trunk/cairoplot.py  2009-07-09 21:57:24 +0000
> +++ trunk/cairoplot.py  2010-07-17 21:38:38 +0000
> @@ -1,4 +1,4 @@
> -#!/usr/bin/env python
> +#!/usr/bin/env python
>  # -*- coding: utf-8 -*-
>
>  # CairoPlot.py
> @@ -288,6 +288,8 @@
>                  series_legend = False,
>                  x_labels = None,
>                  y_labels = None,
> +                 x_formatter = None,
> +                 y_formatter = None,
>                  x_bounds = None,
>                  y_bounds = None,
>                  z_bounds = None,
> @@ -303,6 +305,9 @@
>         self.titles = {}
>         self.titles[HORZ] = x_title
>         self.titles[VERT] = y_title
> +        self.label_formatters = {}
> +        self.label_formatters[HORZ] = x_formatter
> +        self.label_formatters[VERT] = y_formatter
>         self.max_value = {}
>         self.axis = axis
>         self.discrete = discrete
> @@ -396,19 +401,17 @@
>             self.errors[VERT] = [errory]
>
>     def calc_labels(self):
> -        if not self.labels[HORZ]:
> -            amplitude = self.bounds[HORZ][1] - self.bounds[HORZ][0]
> -            if amplitude % 10: #if horizontal labels need floating points
> -                self.labels[HORZ] = ["%.2lf" % (float(self.bounds[HORZ][0]
> + (amplitude * i / 10.0))) for i in range(11) ]
> -            else:
> -                self.labels[HORZ] = ["%d" % (int(self.bounds[HORZ][0] +
> (amplitude * i / 10.0))) for i in range(11) ]
> -        if not self.labels[VERT]:
> -            amplitude = self.bounds[VERT][1] - self.bounds[VERT][0]
> -            if amplitude % 10: #if vertical labels need floating points
> -                self.labels[VERT] = ["%.2lf" % (float(self.bounds[VERT][0]
> + (amplitude * i / 10.0))) for i in range(11) ]
> -            else:
> -                self.labels[VERT] = ["%d" % (int(self.bounds[VERT][0] +
> (amplitude * i / 10.0))) for i in range(11) ]
> -
> +        for key in (HORZ, VERT):
> +            if not self.labels[key]:
> +                amplitude = self.bounds[key][1] - self.bounds[key][0]
> +                labels = (self.bounds[key][0] + (amplitude * i / 10.0) for
> i in range(11))
> +                if self.label_formatters[key]:
> +                    self.labels[key] = [self.label_formatters[key](label)
> for label in labels]
> +                elif amplitude % 10: #if horizontal labels need floating
> points
> +                    self.labels[key] = ["%.2lf" % float(label) for label
> in labels]
> +                else:
> +                    self.labels[key] = ["%d" % int(label) for label in
> labels]
> +
>     def calc_extents(self, direction):
>         self.context.set_font_size(self.font_size * 0.8)
>         self.max_value[direction] = max(self.context.text_extents(item)[2]
> for item in self.labels[direction])
> @@ -510,9 +513,10 @@
>         if self.titles[VERT]:
>             title_width,title_height =
> cr.text_extents(self.titles[VERT])[2:4]
>             cr.move_to( self.dimensions[HORZ] - self.borders[HORZ] +
> title_height/2, self.dimensions[VERT]/2 - title_width/2)
> +            cr.save()
>             cr.rotate( math.pi/2 )
>             cr.show_text( self.titles[VERT] )
> -            cr.rotate( -math.pi/2 )
> +            cr.restore()
>
>     def render_grid(self):
>         cr = self.context
> @@ -544,21 +548,29 @@
>         cr = self.context
>         step = float( self.plot_width ) / ( len( self.labels[HORZ] ) - 1 )
>         x = self.borders[HORZ]
> +        y = self.dimensions[VERT] - self.borders[VERT] + 5
> +
> +        # store rotation matrix from the initial state
> +        rotation_matrix = cr.get_matrix()
> +        rotation_matrix.rotate(self.x_label_angle)
> +
> +        cr.set_source_rgba(*self.label_color)
> +
>         for item in self.labels[HORZ]:
> -            cr.set_source_rgba(*self.label_color)
>             width = cr.text_extents(item)[2]
> -            cr.move_to(x, self.dimensions[VERT] - self.borders[VERT] + 5)
> -            cr.rotate(self.x_label_angle)
> +            cr.move_to(x, y)
> +            cr.save()
> +            cr.set_matrix(rotation_matrix)
>             cr.show_text(item)
> -            cr.rotate(-self.x_label_angle)
> +            cr.restore()
>             x += step
>
>     def render_vert_labels(self):
>         cr = self.context
>         step = ( self.plot_height ) / ( len( self.labels[VERT] ) - 1 )
>         y = self.plot_top
> +        cr.set_source_rgba(*self.label_color)
>         for item in self.labels[VERT]:
> -            cr.set_source_rgba(*self.label_color)
>             width = cr.text_extents(item)[2]
>             cr.move_to(self.borders[HORZ] - width - 5,y)
>             cr.show_text(item)
> @@ -1283,12 +1295,14 @@
>                  y_labels = None,
>                  x_bounds = None,
>                  y_bounds = None,
> -                 series_colors = None):
> +                 series_colors = None,
> +                 value_formatter = None):
>
>         BarPlot.__init__(self, surface, data, width, height, background,
> border,
>                          display_values, grid, rounded_corners, stack,
> three_dimension,
>                          x_labels, y_labels, x_bounds, y_bounds,
> series_colors, VERT)
>         self.series_labels = series_labels
> +        self.value_formatter = value_formatter or str
>
>     def calc_vert_extents(self):
>         self.calc_extents(VERT)
> @@ -1385,20 +1399,22 @@
>         self.context.set_font_size(self.font_size * 0.8)
>         if self.stack:
>             for i,group in enumerate(self.series):
> +                strvalue = self.value_formatter(value)
>                 value = sum(group.to_list())
> -                width = self.context.text_extents(str(value))[2]
> +                width = self.context.text_extents(strvalue)[2]
>                 x = self.borders[HORZ] + (i+0.5)*self.steps[HORZ] +
> (i+1)*self.space - width/2
>                 y = value*self.steps[VERT] + 2
>                 self.context.move_to(x, self.plot_top-y)
> -                self.context.show_text(str(value))
> +                self.context.show_text(strvalue)
>         else:
>             for i,group in enumerate(self.series):
>                 inner_step = self.steps[HORZ]/len(group)
>                 x0 = self.borders[HORZ] + i*self.steps[HORZ] +
> (i+1)*self.space
>                 for number,data in enumerate(group):
> -                    width =
> self.context.text_extents(str(data.content))[2]
> +                    strvalue = self.value_formatter(data.content)
> +                    width = self.context.text_extents(strvalue)[2]
>                     self.context.move_to(x0 + 0.5*inner_step - width/2,
> self.plot_top - data.content*self.steps[VERT] - 2)
> -                    self.context.show_text(str(data.content))
> +                    self.context.show_text(strvalue)
>                     x0 += inner_step
>
>     def render_plot(self):
> @@ -1974,6 +1990,8 @@
>                  series_legend = False,
>                  x_labels = None,
>                  y_labels = None,
> +                 x_formatter = None,
> +                 y_formatter = None,
>                  x_bounds = None,
>                  y_bounds = None,
>                  z_bounds = None,
> @@ -1999,6 +2017,7 @@
>
>     plot = ScatterPlot( name, data, errorx, errory, width, height,
> background, border,
>                         axis, dash, discrete, dots, grid, series_legend,
> x_labels, y_labels,
> +                        x_formatter, y_formatter,
>                         x_bounds, y_bounds, z_bounds, x_title, y_title,
> series_colors, circle_colors )
>     plot.render()
>     plot.commit()
> @@ -2215,7 +2234,8 @@
>                       y_labels = None,
>                       x_bounds = None,
>                       y_bounds = None,
> -                      colors = None):
> +                      colors = None,
> +                      value_formatter = None):
>     #TODO: Fix docstring for vertical_bar_plot
>     '''
>         - Function to generate vertical Bar Plot Charts.
> @@ -2237,6 +2257,7 @@
>         x_labels, y_labels - lists of strings containing the horizontal and
> vertical labels for the axis;
>         x_bounds, y_bounds - tuples containing the lower and upper value
> bounds for the data to be plotted;
>         colors - List containing the colors expected for each of the bars.
> +        value_formatter - if present, this function will be called with
> the value, and the string it returns will be used
>
>         - Example of use
>
> @@ -2246,7 +2267,7 @@
>
>     plot = VerticalBarPlot(name, data, width, height, background, border,
>                            display_values, grid, rounded_corners, stack,
> three_dimension,
> -                           series_labels, x_labels, y_labels, x_bounds,
> y_bounds, colors)
> +                           series_labels, x_labels, y_labels, x_bounds,
> y_bounds, colors, value_formatter)
>     plot.render()
>     plot.commit()
>
>
> === modified file 'trunk/seriestests.py'
> --- trunk/seriestests.py        2009-07-09 21:57:24 +0000
> +++ trunk/seriestests.py        2010-07-17 21:38:38 +0000
> @@ -1,8 +1,16 @@
> -import cairo, math, random
> +import cairo, math, sys
>
>  import cairoplot
>  from series import Series
>
> +# non-random data for needs of visual comparison of changes
> +if '--non-random' in sys.argv:
> +    random = lambda : 1.0
> +    print 'Plotting nonrandom data'
> +else:
> +    import random
> +    random = random.random
> +
>  # Line plotting
>  test_scatter_plot = 1
>  test_dot_line_plot = 1
> @@ -48,8 +56,8 @@
>     f = [math.exp(x) for x in t]
>     g = [10*math.cos(x) for x in t]
>     h = [10*math.sin(x) for x in t]
> -    erx = [0.1*random.random() for x in t]
> -    ery = [5*random.random() for x in t]
> +    erx = [0.1*random() for x in t]
> +    ery = [5*random() for x in t]
>     data = Series({"exp" : [t,f], "cos" : [t,g], "sin" : [t,h]})
>     series_colors = [ (1,0,0), (0,0,0), (0,0,1) ]
>     cairoplot.scatter_plot ( 'cross_r_exponential_series.png', data = data,
> errorx = [erx,erx], errory = [ery,ery], width = 800, height = 600, border =
> 20,
> @@ -77,6 +85,12 @@
>     cairoplot.dot_line_plot( 'dot_line_3_series_legend_series.png', data,
> 400, 300, x_labels = x_labels,
>                              axis = True, grid = True, series_legend = True
> )
>
> +    #Speed test, many x_labels
> +    data = range(1000)
> +    x_labels = [str(x) for x in data]
> +    cairoplot.dot_line_plot( 'dot_line_4_many_x_labels.png', data, 14000,
> 300, x_labels = x_labels,
> +                             axis = True, grid = True, series_legend =
> True )
> +
>  if test_function_plot :
>     #Default Plot
>     data = lambda x : x**2
> @@ -142,7 +156,7 @@
>     cairoplot.vertical_bar_plot ( 'vbar_8_hy_labels_series.png', data, 600,
> 200, border = 20, display_values = True, grid = True, x_labels = x_labels,
> y_labels = y_labels )
>
>     #Large data set
> -    data = Series([[10*random.random()] for x in range(50)])
> +    data = Series([[10*random()] for x in range(50)])
>     x_labels = ["large label name oh my god it's big" for x in data]
>     cairoplot.vertical_bar_plot ( 'vbar_9_large_series.png', data, 1000,
> 800, border = 20, grid = True, rounded_corners = True, x_labels = x_labels )
>
> @@ -182,7 +196,7 @@
>     cairoplot.horizontal_bar_plot ( 'hbar_8_hy_labels_series.png', data,
> 600, 200, border = 20, series_labels = series_labels, display_values = True,
> grid = True, x_labels = x_labels, y_labels = y_labels )
>
>     #Large data set
> -    data = Series([[10*random.random()] for x in range(25)])
> +    data = Series([[10*random()] for x in range(25)])
>     x_labels = ["large label name oh my god it's big" for x in data]
>     cairoplot.horizontal_bar_plot ( 'hbar_9_large_series.png', data, 1000,
> 800, border = 20, grid = True, rounded_corners = True, x_labels = x_labels )
>
> @@ -242,8 +256,8 @@
>     f = [math.exp(x) for x in t]
>     g = [10*math.cos(x) for x in t]
>     h = [10*math.sin(x) for x in t]
> -    erx = [0.1*random.random() for x in t]
> -    ery = [5*random.random() for x in t]
> +    erx = [0.1*random() for x in t]
> +    ery = [5*random() for x in t]
>     data = Series({"exp" : [t,f], "cos" : [t,g], "sin" : [t,h]})
>     series_colors = [ (1,0,0), (0,0,0) ]
>     cairoplot.scatter_plot ( 'scatter_color_themes_series.png', data =
> data, errorx = [erx,erx], errory = [ery,ery], width = 800, height = 600,
> border = 20,
>
> === added directory 'trunk/testscripts'
> === added file 'trunk/testscripts/compare.sh'
> --- trunk/testscripts/compare.sh        1970-01-01 00:00:00 +0000
> +++ trunk/testscripts/compare.sh        2010-07-17 21:38:38 +0000
> @@ -0,0 +1,22 @@
> +#!/bin/sh
> +
> +if [ $# -ne 3 ]
> +then
> +    echo "Compare .png files in two directories"
> +    echo "Usage: ./compare.sh path1 path2 diffdir"
> +    echo "Example: ./compare.sh . ../other ./diff"
> +    exit
> +fi
> +
> +for dir in $1 $2
> +do
> +  for i in $dir/*.png
> +  do
> +    convert $i $i.tiff
> +  done
> +done
> +
> +for i in `(cd $1; ls *.tiff)`
> +do
> +    perceptualdiff $1/$i $2/$i -output $3/$i.ppm
> +done
>
> === added file 'trunk/testscripts/timeit.sh'
> --- trunk/testscripts/timeit.sh 1970-01-01 00:00:00 +0000
> +++ trunk/testscripts/timeit.sh 2010-07-17 21:38:38 +0000
> @@ -0,0 +1,3 @@
> +#!/bin/sh
> +DIRNAME=`dirname $0`
> +(cd $DIRNAME/..; python -m timeit -n 1 -v "import seriestests")
>
>
>

Reply

Revision history for this message

Michael Rooney (mrooney) wrote on 2010-07-17:

#

> Hello there,
>
> As I wasn't too busy, i just reviewed the code. The only thing I didn't get
> was the use of this compare.sh script. Why would you like to compare png
> files?
>
> Great code btw, merging it soon.
>

Thanks! That is showing up because it is including the changes from the speedup branch made by Karel, since this branch is a branch of that branch. I believe it was to make sure the generated images after the speedup changes are identical in testing, to what they should be or were before. It may not need to be in version control once you approve the merge, like the timeit.sh script which is just meant to demonstrate the improvements I believe.

If you like and are going to merge that branch anyway, you can merge that one first and I can re-propose a merge based off of that which should be simpler as it would only be r47-49.

Let me know whatever you prefer.

Reply

lp://qastaging/~mrooney/cairoplot/label-formatters updated on 2010-07-17

50. By Michael Rooney on 2010-07-17: make sure value is defined for stacked vertical bar plots

Revision history for this message

Michael Rooney (mrooney) wrote on 2010-07-17:

#

Just a note, I committed r50 to fix stacked vertical plots which I likely broke.

Reply

Unmerged revisions

50. By Michael Rooney on 2010-07-17: make sure value is defined for stacked vertical bar plots
49. By Michael Rooney on 2010-07-11: fix centering of formatted values in a vertical bar plot
48. By Michael Rooney on 2010-07-11: add support for value formatters to a vertical bar plot
47. By Michael Rooney on 2010-07-11: add support for x/y label formatters for a scatter plot

Cairo Plot

Merge lp://qastaging/~mrooney/cairoplot/label-formatters into lp://qastaging/cairoplot

Commit message

Description of the change

Unmerged revisions

Preview Diff

Subscribers