Re: Patch to add axis position property

[Shai Ayal]

Hi Peter,
I have a few comments:

On 8/9/07, Peter Gustafson <address@hidden> wrote:
> Hi all,
>
> Attached is a patch which adds axis position properties for a plotted
> line, ie corresponding to x2y2 etc for gnuplot and eventually plotyy
> from matlab.  Although certainly not a complete implementation (For
> example I've not yet determined how to set limits based on the
> options).  However it is usable in a very basic way and lays a
> foundation for further work.  I'd like to have it evaluated for inclusion.
>
> The property ends up like this:
>
> octave:1> h=plot(0); get(h)
> ans =
> {
>   type = line
>  ...
>   xaxislocation = bottom
>   yaxislocation = left
> }

This properties are not matlab compatible. The way matlab does plotyy
is to create two overlapping axes, and have each line be a chilld of
the appropriate axes.
I think it's OK to have octave as a superset of matlab, but I can't
see a way in which to reconsile these two approaches.

> Options for y are left and right, for x are top and bottom.
> It is currently implemented to append the "axes xXyY" in the
> usingclause.  There is probably a better place to put it.
>
> Please review, thanks.
>
> Pete Gustafson
>
>
>
>
> Index: scripts/plot/__go_draw_axes__.m
> ===================================================================
> RCS file: /cvs/octave/scripts/plot/__go_draw_axes__.m,v
> retrieving revision 1.32
> diff -c -r1.32 __go_draw_axes__.m
> *** scripts/plot/__go_draw_axes__.m     24 Jul 2007 19:02:27 -0000      1.32
> --- scripts/plot/__go_draw_axes__.m     9 Aug 2007 17:33:03 -0000
> ***************
> *** 293,298 ****
> --- 293,308 ----
>           endif
>           [style, typ, with] = do_linestyle_command (obj, data_idx, 
> plot_stream);
>           usingclause{data_idx} = "";
> +         if (strcmpi(obj.xaxislocation,"top"))
> +           xaxisloc="x2";
> +         else
> +           xaxisloc="x1";
> +         endif
> +         if (strcmpi(obj.yaxislocation,"right"))
> +           yaxisloc="y2";
> +         else
> +           yaxisloc="y1";
> +         endif
>           if (have_newer_gnuplot || isnan (typ))
>             withclause{data_idx} = sprintf ("with %s linestyle %d",
>                                             style, data_idx);
> ***************
> *** 397,403 ****
>                 [ymin, ymax, yminp] = get_data_limits (ymin, ymax, yminp, 
> ydat);
>               endif
>               data{data_idx} = [xdat, ydat]';
> !             usingclause{data_idx} = "using ($1):($2)";
>             endif
>           endif
>           if (! (have_newer_gnuplot || isempty (with)))
> --- 407,414 ----
>                 [ymin, ymax, yminp] = get_data_limits (ymin, ymax, yminp, 
> ydat);
>               endif
>               data{data_idx} = [xdat, ydat]';
> !             usingclause{data_idx} = sprintf("using ($1):($2) axes %s%s",
> !                                             xaxisloc,yaxisloc);
>             endif
>           endif
>           if (! (have_newer_gnuplot || isempty (with)))
> Index: src/graphics.cc
> ===================================================================
> RCS file: /cvs/octave/src/graphics.cc,v
> retrieving revision 1.23
> diff -c -r1.23 graphics.cc
> *** src/graphics.cc     24 Jul 2007 19:02:27 -0000      1.23
> --- src/graphics.cc     9 Aug 2007 17:33:05 -0000
> ***************
> *** 1587,1593 ****
>       markeredgecolor ("auto"),
>       markerfacecolor ("none"),
>       markersize (1),
> !     keylabel ("")
>   { }
>
>   void
> --- 1587,1595 ----
>       markeredgecolor ("auto"),
>       markerfacecolor ("none"),
>       markersize (1),
> !     keylabel (""),
> !     xaxislocation ("bottom"),
> !     yaxislocation ("left")
>   { }
>
>   void
> ***************
> *** 1634,1639 ****
> --- 1636,1645 ----
>       markersize = val;
>     else if (name.compare ("keylabel"))
>       keylabel = val;
> +   else if (name.compare ("xaxislocation"))
> +     xaxislocation = val;
> +   else if (name.compare ("yaxislocation"))
> +     yaxislocation = val;
>     else
>       {
>         modified = false;
> ***************
> *** 1668,1673 ****
> --- 1674,1681 ----
>     m.assign ("markerface", markerfacecolor);
>     m.assign ("markersize", markersize);
>     m.assign ("keylabel", keylabel);
> +   m.assign ("xaxislocation", xaxislocation);
> +   m.assign ("yaxislocation", yaxislocation);
>
>     return m;
>   }
> ***************
> *** 1715,1720 ****
> --- 1723,1732 ----
>       retval = markersize;
>     else if (name.compare ("keylabel"))
>       retval = keylabel;
> +   else if (name.compare ("xaxislocation"))
> +     retval = keylabel;
> +   else if (name.compare ("yaxislocation"))
> +     retval = keylabel;
>     else
>       warning ("get: invalid property `%s'", name.c_str ());
>
> ***************
> *** 1740,1745 ****
> --- 1752,1759 ----
>     m["markerfacecolor"] = "none";
>     m["markersize"] = 1;
>     m["keylabel"] = "";
> +   m["xaxislocation"] = "bottom";
> +   m["yaxislocation"] = "left";
>
>     return m;
>   }
> Index: src/graphics.h
> ===================================================================
> RCS file: /cvs/octave/src/graphics.h,v
> retrieving revision 1.7
> diff -c -r1.7 graphics.h
> *** src/graphics.h      24 Jul 2007 19:02:27 -0000      1.7
> --- src/graphics.h      9 Aug 2007 17:33:05 -0000
> ***************
> *** 1357,1362 ****
> --- 1357,1364 ----
>       OCTAVE_GRAPHICS_PROPERTY (octave_value, markerfacecolor);
>       OCTAVE_GRAPHICS_PROPERTY (octave_value, markersize);
>       OCTAVE_GRAPHICS_PROPERTY (octave_value, keylabel);
> +     OCTAVE_GRAPHICS_PROPERTY (octave_value, xaxislocation);
> +     OCTAVE_GRAPHICS_PROPERTY (octave_value, yaxislocation);

All of the above properties (I know you aren't responsible for all of
them) should be of the type radio_properties. This will ensure that
you can only set them to on of the predifined legal values.

>       static std::string go_name;
>     };
>
> 2007-08-09  Peter Gustafson  <address@hidden>
>
>         * scripts/plot/__go_draw_axes__.m: Add axis position to the 
> usingclause.
>         * src/graphics.h, src/graphics.cc: Add properties for 
> [xy]axislocation.
>