[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: plot templates and options lists for set, plot etc.
From: |
Thorsten Meyer |
Subject: |
Re: plot templates and options lists for set, plot etc. |
Date: |
Mon, 04 Jan 2010 13:01:01 +0100 |
User-agent: |
Mozilla-Thunderbird 2.0.0.22 (X11/20090706) |
Thorsten Meyer wrote:
> Thanks for your feedback (and your offline reminder :-) ).
>
> Included you will find an updated patch. See comments below.
>
> John W. Eaton wrote:
>> On 16-Sep-2009, Thorsten Meyer wrote:
>>
>> | Thorsten Meyer wrote:
>> | > Here is a patch implementing struct and cell array arguments for the set
>> function.
>> | >
>> | > I have impplemented the matlab behaviour (as I understand it ...). See
>> the tests
>> | > added to the patch for the details.
>> | >
>> | > Could somebody please review the patch (what I do in the sources is
>> still a lot
>> | > of trial and error):
>> | > - Have I got the types of the various variables right?
>>
>> | > - Is it ok to add doxygen style comments to the new methods? (the
>> | > doxygen docs
>>
>> I don't use Doxygen, so I'm unlikely to remember to add the markup in
>> the comments I write. If we decide to do this, then I think we should
>> try to do it on a more global scale.
> I removed the Doxygen markup for now.
>
>> | > - What about the coding style?
>> | > - Are the added methods for the graphics_object ok (also where I placed
>> the
>> | > definitions)?
>>
>> I try to keep the order of the declarations in the .h file the same as
>> the order in the .cc file. If you've done that, then it is probably
>> OK.
> I fixed the order to be consistent in .h, .cc and the documentation string.
>
>> | +//! Set properties given in two cell arrays containing names and values.
>> | +void
>> | +graphics_object::set (const Array<std::string> names,
>> | + const Cell values, octave_idx_type row)
>>
>> I think this should be
>>
>> graphics_object::set (const Array<std::string>& names,
>> const Cell& values, octave_idx_type row)
> done.
>
>> | + if (! (names.numel () == values.columns ()))
>>
>> Why use
>>
>> ! (x == y)
>>
>> instead of
>>
>> x != y
>>
>> ?
> I changed it.
>
>> | +void
>> | +graphics_object::set (const Octave_map m)
>>
>> This should be
>>
>> void
>> graphics_object::set (const Octave_map& m)
> Done
>
>> | +//! Set a property to a value or to its (factory) default value.
>> | +void graphics_object::set_value_or_default (caseless_str& name,
>> | + octave_value& val)
>>
>> I think this should be
>>
>> void
>> graphics_object::set_value_or_default (const caseless_str& name,
>> const octave_value& val)
>>
>> unless the function needs to modify NAME and VAL. It doesn't appear
>> need to.
>>
>> | + if (val.is_string ())
>> | + {
>> | + caseless_str tval = val.string_value ();
>> | +
>> | + if (tval.compare ("default"))
>> | + val = get_default (name);
>> | + else if (tval.compare ("factory"))
>> | + val = get_factory_default (name);
>> | + }
>> | +
>> | + if (error_state)
>> | + return;
>> | +
>> | + rep->set (name, val);
>> | +}
>>
>> With the non-const VAL, if VAL is a string, then it will also be
>> changed in the caller. Is that really what you want? I think it
>> would be better to define a local temporary value inside the
>> function.
> I changed the method arguments as you suggested and blew up the conditional a
> bit to avoid copying val.
>
here is a kind reminder for the patch above and an updated patch. Can it be
applied as it is?
thanks
Thorsten
# HG changeset patch
# User Thorsten Meyer <address@hidden>
# Date 1262599429 -3600
# Node ID 945826d8f9aa27779958e20c215800d763992340
# Parent 5813ec5077b568b705a536907d219ac0e44b76e5
Fix set function to allow cell and struct arguments.
diff -r 5813ec5077b5 -r 945826d8f9aa src/ChangeLog
--- a/src/ChangeLog Mon Jan 04 03:00:19 2010 -0500
+++ b/src/ChangeLog Mon Jan 04 11:03:49 2010 +0100
@@ -0,0 +1,8 @@
+2010-01-04 Thorsten Meyer <address@hidden>
+
+ * graphics.cc (Fset), graphics.cc (graphics_object::set): Add
+ support for struct and cell arguments.
+
+ * graphics.h.in (graphics_objects::set): Add prototypes for
+ calling with struct respectively cell arguments.
+
diff -r 5813ec5077b5 -r 945826d8f9aa src/graphics.cc
--- a/src/graphics.cc Mon Jan 04 03:00:19 2010 -0500
+++ b/src/graphics.cc Mon Jan 04 11:03:49 2010 +0100
@@ -1395,6 +1395,7 @@
}
}
+// Set properties given as a cs-list of name, value pairs
void
graphics_object::set (const octave_value_list& args)
{
@@ -1412,20 +1413,10 @@
{
octave_value val = args(i+1);
- if (val.is_string ())
- {
- caseless_str tval = val.string_value ();
-
- if (tval.compare ("default"))
- val = get_default (name);
- else if (tval.compare ("factory"))
- val = get_factory_default (name);
- }
+ set_value_or_default (name, val);
if (error_state)
break;
-
- rep->set (name, val);
}
else
error ("set: expecting argument %d to be a property name", i);
@@ -1435,6 +1426,150 @@
error ("set: invalid number of arguments");
}
+/*
+%!# test set with name, value pairs
+%!test
+%! set(gcf, "visible", "off");
+%! h = plot (1:10, 10:-1:1);
+%! set (h, "linewidth", 10, "marker", "x");
+%! assert (get (h, "linewidth"), 10);
+%! assert (get (h, "marker"), "x");
+*/
+
+// Set properties given in two cell arrays containing names and values.
+void
+graphics_object::set (const Array<std::string>& names,
+ const Cell& values, octave_idx_type row)
+{
+ if (names.numel () != values.columns ())
+ {
+ error("set: number of names must match number of value columns (%d !=
%d)",
+ names.numel (), values.columns ());
+ }
+
+ octave_idx_type k = names.columns ();
+
+ for (octave_idx_type column = 0; column < k; column++)
+ {
+ caseless_str name = names(column);
+ octave_value val = values(row, column);
+
+ set_value_or_default (name, val);
+
+ if (error_state)
+ break;
+ }
+}
+
+/*
+%!# test set with cell array arguments
+%!test
+%! set (gcf, "visible", "off");
+%! h = plot (1:10, 10:-1:1);
+%! set (h, {"linewidth", "marker"}, {10, "x"});
+%! assert (get(h, "linewidth"), 10);
+%! assert (get(h, "marker"), "x");
+
+%!# test set with multiple handles and cell array arguments
+%!test
+%! set (gcf, "visible", "off");
+%! h = plot (1:10, 10:-1:1, 1:10, 1:10);
+%! set (h, {"linewidth", "marker"}, {10, "x"; 5, "o"});
+%! assert (get (h, "linewidth"), {10; 5});
+%! assert (get (h, "marker"), {"x"; "o"});
+%! set (h, {"linewidth", "marker"}, {10, "x"});
+%! assert (get (h, "linewidth"), {10; 10});
+%! assert (get (h, "marker"), {"x"; "x"});
+
+%!error <set: number of graphics handles must match number of value rows>
+%! set (gcf, "visible", "off");
+%! h = plot (1:10, 10:-1:1, 1:10, 1:10);
+%! set (h, {"linewidth", "marker"}, {10, "x"; 5, "o"; 7, "."});
+
+%!error <set: number of names must match number of value columns>
+%! set (gcf, "visible", "off");
+%! h = plot (1:10, 10:-1:1, 1:10, 1:10);
+%! set (h, {"linewidth"}, {10, "x"; 5, "o"});
+*/
+
+// Set properties given in a struct array
+void
+graphics_object::set (const Octave_map& m)
+{
+ for (Octave_map::const_iterator p = m.begin ();
+ p != m.end (); p++)
+ {
+ caseless_str name = m.key (p);
+
+ octave_value val = octave_value (m.contents (p).elem (m.numel () - 1));
+
+ set_value_or_default (name, val);
+
+ if (error_state)
+ break;
+ }
+}
+
+/*
+%!# test set with struct arguments
+%!test
+%! set (gcf, "visible", "off");
+%! h = plot (1:10, 10:-1:1);
+%! set (h, struct ("linewidth", 10, "marker", "x"));
+%! assert (get (h, "linewidth"), 10);
+%! assert (get (h, "marker"), "x");
+%! h = plot (1:10, 10:-1:1, 1:10, 1:10);
+%! set (h, struct ("linewidth", {5, 10}));
+%! assert (get(h, "linewidth"), {10; 10});
+*/
+
+// Set a property to a value or to its (factory) default value.
+void graphics_object::set_value_or_default (const caseless_str& name,
+ const octave_value& val)
+{
+ if (val.is_string ())
+ {
+ caseless_str tval = val.string_value ();
+
+ octave_value default_val;
+
+ if (tval.compare ("default"))
+ {
+ default_val = get_default (name);
+
+ if (error_state)
+ return;
+
+ rep->set (name, default_val);
+ }
+ else if (tval.compare ("factory"))
+ {
+ default_val = get_factory_default (name);
+
+ if (error_state)
+ return;
+
+ rep->set (name, default_val);
+ }
+ else
+ rep->set (name, val);
+ }
+ else
+ rep->set (name, val);
+}
+
+/*
+%!# test setting of default values
+%!test
+%! set (gcf, "visible", "off");
+%! h = plot (1:10, 10:-1:1);
+%! set (0, "defaultlinelinewidth", 20);
+%! set (h, "linewidth", "default");
+%! assert (get (h, "linewidth"), 20);
+%! set (h, "linewidth", "factory");
+%! assert (get (h, "linewidth"), 0.5);
+*/
+
static double
make_handle_fraction (void)
{
@@ -4570,9 +4705,35 @@
DEFUN (set, args, ,
"-*- texinfo -*-\n\
address@hidden {Built-in Function} {} set (@var{h}, @var{p}, @var{v},
@dots{})\n\
-Set the named property value or vector @var{p} to the value @var{v}\n\
-for the graphics handle @var{h}.\n\
address@hidden {Built-in Function} {} set (@var{h}, @var{property},
@var{value}, @dots{})\n\
address@hidden {Built-in Function} {} set (@var{h}, @var{properties},
@var{values})\n\
address@hidden {Built-in Function} {} set (@var{h}, @var{pv})\n\
+Set named property values for the graphics handle (or vector of graphics\n\
+handles) @var{h}.\n\
+There are three ways how to give the property names and values:\n\
+\n\
address@hidden
address@hidden as a comma separated list of @var{property}, @var{value} pairs\n\
+\n\
+Here, each @var{property} is a string containing the property name, each\n\
address@hidden is a value of the appropriate type for the property.\n\
address@hidden as a cell array of strings @var{properties} containing property
names\n\
+and a cell array @var{values} containing property values.\n\
+\n\
+In this case, the number of columns of @var{values} must match the number of\n\
+elements in @var{properties}. The first column of @var{values} contains
values\n\
+for the first entry in @var{properties} etc.. The number of rows of
@var{values}\n\
+must be 1 or match the number of elements of @var{h}. In the first case,
each\n\
+handle in @var{h} will be assigned the same values. In the latter case, the\n\
+first handle in @var{h} will be assigned the values from the first row of\n\
address@hidden and so on.\n\
address@hidden as a structure array @var{pv}\n\
+\n\
+Here, the field names of @var{pv} represent the property names, and the
field\n\
+values give the property values. In contrast to the previous case, all\n\
+elements of @var{pv} will be set in all handles in @var{h} independent of\n\
+the dimensions of @var{pv}.\n\
address@hidden itemize\n\
@end deftypefn")
{
gh_manager::autolock guard;
@@ -4583,28 +4744,62 @@
if (nargin > 0)
{
+ // get vector of graphics handles
ColumnVector hcv (args(0).vector_value ());
if (! error_state)
{
bool request_drawnow = false;
+ // loop over graphics objects
for (octave_idx_type n = 0; n < hcv.length (); n++)
{
graphics_object obj = gh_manager::get_object (hcv(n));
if (obj)
{
- obj.set (args.splice (0, 1));
-
- request_drawnow = true;
+ if (nargin == 3 && args(1).is_cellstr ()
+ && args(2).is_cell ())
+ {
+ if (args(2).cell_value ().rows () == 1)
+ {
+ obj.set (args(1).cellstr_value (),
+ args(2).cell_value (), 0);
+ }
+ else if (hcv.length () == args(2).cell_value ().rows ())
+ {
+ obj.set (args(1).cellstr_value (),
+ args(2).cell_value (), n);
+ }
+ else
+ {
+ error("set: number of graphics handles must match
number of value rows (%d != %d)",
+ hcv.length (), args(2).cell_value ().rows ());
+ break;
+
+ }
+ }
+ else if (nargin == 2 && args(1).is_map ())
+ {
+ obj.set (args(1).map_value ());
+ }
+ else
+ {
+ obj.set (args.splice (0, 1));
+ request_drawnow = true;
+ }
}
else
{
error ("set: invalid handle (= %g)", hcv(n));
break;
}
- }
+
+ if (error_state)
+ break;
+
+ request_drawnow = true;
+ }
if (! error_state && request_drawnow)
Vdrawnow_requested = true;
diff -r 5813ec5077b5 -r 945826d8f9aa src/graphics.h.in
--- a/src/graphics.h.in Mon Jan 04 03:00:19 2010 -0500
+++ b/src/graphics.h.in Mon Jan 04 11:03:49 2010 +0100
@@ -2059,6 +2059,14 @@
void set (const octave_value_list& args);
+ void set (const Array<std::string>& names, const Cell& values,
+ octave_idx_type row);
+
+ void set (const Octave_map& m);
+
+ void set_value_or_default (const caseless_str& name,
+ const octave_value& val);
+
void set_defaults (const std::string& mode) { rep->set_defaults (mode); }
octave_value get (bool all = false) const { return rep->get (all); }
- Re: plot templates and options lists for set, plot etc.,
Thorsten Meyer <=