bugGNU Octave - Bugs: bug #37290, gnuplot / demo plotyy followed by...

 
 

bug #37290: gnuplot / demo plotyy followed by sombrero errors out

Submitter:  Philip Nienhuis <philipnienhuis>
Submitted:  Thu 06 Sep 2012 08:35:17 PM UTC
   
 
Category:  Plotting with gnuplot Severity:  2 - Minor
Priority:  5 - Normal Item Group:  Regression
Status:  Works For Me Assigned to:  None
Originator Name:  Philip Nienhuis Open/Closed:  * Closed
Release:  * dev Operating System:  * Any
Fixed Release:  None Planned Release:  None
* Mandatory Fields

Add a New Comment Rich Markup
   

Jump to the original submission

Tue 21 May 2013 05:37:31 AM UTC, comment #40: 

Its been my practice to include a clf() at the start of each demo.  I did that with the expectation that prior actions would not impact the demo.  Looks like that isn't sufficient.  So, I'd say "yes", a reset(gcf()) should also be included.

Ben Abbott <bpabbott>
Group Member
Tue 21 May 2013 05:23:09 AM UTC, comment #39: 

About the hold on in the demo...  Should we be automatically resetting plot properties (including the hold state) to their defaults at the start of the demo function itself?

John W. Eaton <jwe>
Group administrator
Mon 20 May 2013 09:41:09 AM UTC, comment #38: 

Dan,

The example you gave works as expected.  To see if the default axes linewidth has changed, you'll need to create a new axes object, or render a new plot (with no "hold on").  Try,


set (gcf (), 'defaultaxeslinewidth', 3)
plot (rand (3))
reset (gcf ())
plot (rand (3))


I like your suggestion.  Attach the patch to the other changeset. I'll close this one.

Ben

Ben Abbott <bpabbott>
Group Member
Mon 20 May 2013 09:22:09 AM UTC, comment #37: 

From https://savannah.gnu.org/bugs/?37290#comment14

Try something like:


set (gcf, "defaultaxeslinewidth", 3)
plot (1:50)
reset (gcf)


But I see now that there doesn't appear to be any "reset" in demo plotyy or sombrero.  Should we close this bug and attach my previous changeset

https://savannah.gnu.org/bugs/download.php?file_id=28111

to https://savannah.gnu.org/bugs/?35511 ?

Dan

PS: In the images you attached is also a difference between the line colors for the sinusoids.

Dan Sebald <sebald>
Mon 20 May 2013 06:00:46 AM UTC, comment #36: 

Dan,

The only problem I see with respect to the un-patched default branch is that the error from setting a read-only property is misleading.  For example, Matlab gives a more useful error.


set (gca, 'tightinset', [0 0])
Error using set
Attempt to modify a property that is read-only.
Object Name: axes
Property Name: 'TightInset'.


Am I missing something?

Ben Abbott <bpabbott>
Group Member
Mon 20 May 2013 05:10:29 AM UTC, comment #35: 

Ben,

That error is what happens after the patch is applied and a "reset (gcf)" is input.  The way the code is set up, read-only variables are in the factory defaults:


property_list::pval_map_type
axes::properties::factory_defaults (void)
{
  property_list::pval_map_type m = base_properties::factory_defaults ();

  m["position"] = default_axes_position ();
  m["box"] = "on";
{snip}
  m["tightinset"] = Matrix (1, 4, 0.0);
{snip}
  m["autopos_tag"] = "none";

  return m;
}


but they are not in this if-else "set" structure:


void
axes::properties::set (const caseless_str& pname_arg, const octave_value& val)
{
  const std::set<std::string>& pnames = all_property_names ();

  caseless_str pname = validate_property_name ("get", go_name, pnames, pname_arg);

  if (error_state)
    return;

  if (pname.compare ("position"))
    set_position (val);
  else if (pname.compare ("box"))
    set_box (val);
  else if (pname.compare ("colororder"))
{snip: no 'tightinset' in this list}
  else if (pname.compare ("autopos_tag"))
    set_autopos_tag (val);
  else
    base_properties::set (pname, val);
}


The point is that there is no way of knowing which are read-only variables with this construct, so if one restores the factory settings the only thing is to know beforehand which read-only variable names to condition out of the reset.  Not difficult to do, but again it feels like real patchwork for only marginal gain.

If you want to replicate the issue, try


>> get (gca, 'tightinset')
ans =

   0   0   0   0

>> set (gca, 'tightinset', [0 0 0 0])
error: set: unknown property "tightinset"


Regarding the 'set(gca, "defaultlinewidth", 2.3)', you are correct.  Maybe I was confused by the axes "linewidth", but as written the "width" is checked in the "line" object.  I will look at that again.  Thanks.

Dan Sebald <sebald>
Mon 20 May 2013 12:37:55 AM UTC, comment #34: 

John,

Regarding the "demo plotyy" + "sombrero" problem,  the cause is the "hold on" statement in the fourth demo.


%!demo
%! clf
%! hold on
%! t = (0:0.1:9);
%! x = sin (t);
%! y = 5 * cos (t);
%! [hax, h1, h2] = plotyy (t, x, t, y);
%! [~, h3, h4] = plotyy (t+1, x, t+1, y);
%! set ([h3, h4], "linestyle", "--")
%! xlabel (hax(1), 'xlabel')
%! title (hax(2), 'title')
%! ylabel (hax(1), 'Left axis is Blue')
%! ylabel (hax(2), 'Right axis is Green')


I tried the example below on both Matlab R2013a and Octave.


x = 0:0.01:10;
plotyy (x, cos (x), x, sin(x))
hold on
peaks


There are some minor differences in axes scaling, but it appears to me that Octave is working correctly. I've attached the plots I get from Matlab and Octave.

Have I missed something?

Ben



Ben Abbott <bpabbott>
Group Member
Mon 20 May 2013 12:26:19 AM UTC, comment #33: 

Dan,

The syntax below is not valid.


set(gca, "defaultlinewidth", 2.3)


The syntax is ["default", objectname, propertyname].  The objectname is typically "line", and the property name is "linewidth".  Thus, the proper syntax includes two consecutive instances of "line".


set(gca, "defaultlinelinewidth", 2.3)


Regarding the "tightinset" error, I've never seen this.  Can you provide an example script that will trigger this error?


Ben Abbott <bpabbott>
Group Member
Sun 19 May 2013 04:47:57 PM UTC, comment #32: 

Having thought about this for a few days, I've concluded there's too much work here for me to undertake right now.  But I do think there is a potentially great summer student project here, or maybe a group effort code sprint for OctConf 13.  I'll summarize why at the end.

There isn't anything obvious to me that is a simple solution to make this work the way I understand it should work.  The things I initially thought were problematic are actually sort of correct, if perhaps some minor fixes here and there.  (I was kind of thinking about things differently on first inspection.)

John, the attached changeset is probably the best I can do for now.  It doesn't really fix things, more like the old-college-try; might even make things worse.  You can maybe improve on this, but I don't want to spend too much more effort on it because I'm thinking there isn't a good short-term solution.  Basically, the patch resets properties to the factory settings, but it is a little tricky because properties are defined within the derived graphic object classes (not the base graphics object) which means that each graphics object needs to have its own member function for accessing those properties.  Hence, there is some code duplication.  I'm not really happy with the solution, as it sort of works its way in the direction of the children, when I think the more natural hierarchy direction is in the direction of the parent.  The reason some things break is because factory settings apparently aren't always good for the graphics system.

I know that the intermediate "defaultXYZ" isn't (and wasn't) working because there is this bug that currently exists:


>> set(gca, "defaultlinewidth", 2.3)

error: invalid line property 'width'


I've fixed that so that at least there is no longer an error message, but I'm sure the results are dodgy because I didn't do much beyond that.

Also, I don't think there is a way to get rid of the error message about unknown properties when they are readonly ('r' in genprops.awk).  For example:


>> reset(hax)
error: set: unknown property "tightinset"


The reason is that message is generated at the end of a long if-else statement.  The valid settable properties is contained within the following if-else structure and is inaccessible


  const std::set<std::string>& pnames = xproperties.all_property_names ();
  for (p = pm.begin (); p != pm.end (); p++)
    {
      // Read-only properties (i.e., 'r' described in genprops.awk)
      // do not appear within the test structure for valid properties
      // (i.e., can only get read-only props, not set them).  So,
      // first check to validate it is present to avoid an error
      // message.
      caseless_str pname (p->first);
      pname = validate_property_name ("get",
                                      xproperties.graphics_object_name (),
                                      pnames, pname);


The above is the equivalent of "have_property" without getting the list of properties every time a test is desired.  However, that includes all properties, and does not distinguish if the property can be set. Probably the easiest way to fix this is to change the genprops.awk script to calling read-only set_XYZ() but make the associated function empty, i.e., do nothing.

Note further that some of the error messages actually pertain to bad values for the factory settings.  For example:


error: invalid color specification: {auto}|none
error: invalid value for color property "markeredgecolor" (value= {auto}|none)
error: invalid color specification: auto|{none}
error: invalid value for color property "markerfacecolor" (value= auto|{none})


Anyway, what I've done is put into the "set" and "reset" documentation a note stating we are aware of some problems therefore don't submit a bug report.  That way, we can close out this bug report and bug report https://savannah.gnu.org/bugs/?35511 with an eye toward a student project for the release after the June release.

If John can't improve on the attached patch, maybe we should just leave it as is, with some added documentation for "reset" about known bugs.  My opinion is that any more effort to patch things isn't going to amount to much.  I may be wrong.

...

I think conceptually, on the whole, the property management scheme isn't falling into place quite right.  Now, that's not to say the code has wandered off course.  The individual parts are all about right, but it's as though organizing things in a slightly different way would in some conceptual ways and some programming ways simplify matters.  I'll give an example.

I like the use of std::map, and there are macro definitions for all the possible properties.  Furthermore, there are some member functions for determining if a certain graphics object has the property specified. And the property_list is nice, i.e., all these graphics objects have a list containing properties set by the user.  But there is a sort of disconnect between the organization of the properties and the way they are referenced by Octave code.  For example, here is the creation of factory properties:


property_list::plist_map_type
root_figure::init_factory_properties (void)
{
  property_list::plist_map_type plist_map;

  plist_map["figure"] = figure::properties::factory_defaults ();
  plist_map["axes"] = axes::properties::factory_defaults ();
  plist_map["line"] = line::properties::factory_defaults ();

etc.
}


The groupings makes sense, but it unfortunately doesn't retain any hierarchical information.  If things were organized to retain hierarchy, then perhaps the algorithm that does a "get" on the desired variable might be somewhat simple and abstract.  Ben describes how the default syntax works, e.g., "defaultfigureaxeslinelinewidth" at the root_figure level, "defaultaxeslinelinewidth" at the figure level, "defaultlinelinewidth" at the axes level, if I'm understanding correctly.  What if those names were used as the index names for the map list?  Here's a sample.  Say the desired property is the linewidth of some line:

dprop = "linewidth"

level 1 (line) Is there a dprop in my user_set_plist_map[]?  If yes, get user_set_plist_map[dprop].  If no, add prefix "line" to create dprop = "line" + dprop.  Pass dprop to the parent get routine.

level 2 (axes) Is there a dprop in my user_set_plist_map[]?  If yes, get user_set_plist_map[dprop].  If no, add prefix "axes" to create dprop = "axes" + dprop.  Pass dprop to the parent get routine.

level 3 (figure) Is there a dprop in my user_set_plist_map[]?  If yes, get user_set_plist_map[dprop].  If no, add prefix "figure" to create dprop = "figure" + dprop.  Pass dprop to the parent get routine.

level 4 (root_figure) Is there a dprop in my user_set_plist_map[]?  If yes, get user_set_plist_map[dprop].  If no, get factory_plist_map[dprop].

So, at the point of reaching level 4, the index for the map list becomes "figureaxeslinelinewidth".  So, that name coincides with "defaultfigureaxeslinelinewidth" and at the same time utilizes a very generic routine that is almost the same at all levels.  I left the "line", "axes", and "figure" in for clarity, but really that could be replaced by an identity variable, say gobjid.  With that, the routine is the same at all levels, except the root figure level.

One side note: A map with so many indeces might get kind of slow, especially at the root level.  The map could also be done in a hierarchical fashion (similar to multiple levels it currently is), and processing could be made a bit easier by using a delimiter behind the scenes, e.g., "figure:axes:line:linewidth".  That would make parsing the map indeces pretty straightforward.  When it comes time to displaying this parameter, the delimiter symbols could be extracted. So, one could use probably the internal regexp routine to get a cell array consisting of {figure, axes, line, linewidth} and access the property via

user_set_plist_map[p{1}].m[p{2}].m[p{3}].m[p{4}]

I'm guessing that is much faster than trying to index a map list with possibly hundreds of entries.  But above the root_figure level, the parsing probably isn't necessary because there won't be more than a dozen user defined properties.

Well anyway, I hope I've conveyed something that might give John and others some ideas for what I think could be a nice student project.  The reasons I think it's good are:

1) Many of the pieces are already worked out in the current code, so the student wouldn't have to work from a clean slate and could learn a lot by just studying the code for a week or two.

2) It uses some nice standardized features like std::map.  That's something a computer science/software engineer would benefit from learning.

3) The project is somewhat substantial in effort, but not huge.

4) The coding is very object oriented, using things like function inheritance and/or virtual functions.

5) The structure has a hierarchy nature to it.  This feels like it is in the realm of database management, so any student interested in that field could really benefit from a project like this.

I could maybe mentor such a thing, but I won't have much time after the end of May.


(file #28111)

Dan Sebald <sebald>
Fri 17 May 2013 04:02:45 AM UTC, comment #31: 

Well, I'm not sure I've accomplished much other than fixing that small bug with setting defaults.  I.e., this result:


>> set(gca, "defaultlinewidth", 2.3)

error: invalid line property 'width'


The attached patch will clear this error, but it's too simple and I'm not sure it agrees with what you are describing Ben.  Here is a sample of what it produces:


>> haxis = gca
haxis = -6.8962
>> get(haxis, "default")
ans =

  scalar structure containing the fields:


>> set(haxis, "defaultlinewidth", 5)
>> get(haxis, "default")
ans =

  scalar structure containing the fields:

    defaultaxeslinewidth =  5

>> reset(haxis)
>> get(haxis, "default")
ans =

  scalar structure containing the fields:


>>


So, it sort of behaves the way defaults should, but notice the "axes" probably should not be in the name in this case, i.e., it should read "defaultlinewidth", not "defaultaxeslinewidth".  Now this might be a fairly cosmetic thing, but one has to think of where to handle that: in the map itself (i.e., the property is placed in the defaults map as "defaultlinewidth"), or when the map is displayed (i.e., the property name is parsed and "axes" extracted).

Before going that route, I think it might be good to think about this for a few days.  It's probably the case there is too much to tackle in the time frame of a GUI release, and in fact this might make a good Google Summer project with a mercurial branch.  I can't quite understand it all right now.  The root_figure seems solid--at the 0 level is defined all these factory settings, every property and value is there.  And the macros contain all information about possible properties and their possible settings.  But at the other end the properties are gotten via simple functions that return values.  But if I try to use, say "defaultlinelinewidth" at the "axes" level to set that child property value I'm not sure if the user has set that property value directly.  Going backward through the parents seems easier, i.e., if linewidth not mapped, look to parent and if defaultaxeslinewidth not mapped, look to parent and if defaultfigureaxeslinewidth not mapped, look to factoryfigureaxeslinewidth.  But that too I'm not sure about.

I'm going to hold off for a few days, but what I might try for is just setting the property values back to the root_figure factory/default when a "reset(obj)" is done.  For the next release, two ends of the hierarchy sort of working correctly is probably better than having an intermediate level overriding something it shouldn't.

(file #28098)

Dan Sebald <sebald>
Fri 17 May 2013 12:46:48 AM UTC, comment #30: 

To set the default for a figure's children, use the same syntax as for the root, but change the object's handle.


set (gcf (), 'defaultaxeszlim', foo)


Same for axes, but don't forget to "hold all", or else the axes property will be reset.


plot (rand (3))
set (gca (), 'defaultlinelinewidth', 2.3)
hold all
plot (rand (3))



Ben Abbott <bpabbott>
Group Member
Thu 16 May 2013 11:58:06 PM UTC, comment #29: 

Yes, I just figured that out by trying:

>> set(0, "defaultaxeslinewidth", 2.3)
>> plot(1:50)


Small problem here:

>> set(gca, "defaultlinewidth", 2.3)

error: invalid line property 'width'

>> set(gca, "linewidth", 2.3)


OK, I understand it now.

Thanks

Dan Sebald <sebald>
Thu 16 May 2013 11:49:48 PM UTC, comment #28: 

I think what is supposed to happen is


set (0, 'defaultaxeszlim', foo)


sets the global default and


set (figure_handle, 'defaultzlim', foo)


sets the default for the given figure handle.

http://www.mathworks.com/help/matlab/creating_plots/setting-default-property-values.html


John W. Eaton <jwe>
Group administrator
Thu 16 May 2013 11:22:11 PM UTC, comment #27: 

Oh, OK.  And I see there is a way to set defaults:


  void set (const caseless_str& name, const octave_value& value)
  {
    if (name.compare ("default", 7))
      // strip "default", pass rest to function that will
      // parse the remainder and add the element to the
      // default_properties map.
      default_properties.set (name.substr (7), value);
    else
      xproperties.set (name, value);


xproperties is the current properties for the figure, i.e., when a conventional "set(gca, "linewidth", 2.3)" is done.

default_properties, I'm guessing, is supposed to be the default for a new figure or the result of "reset".  It is controlled via "set(gca, "defaultlinewidth", 2.3)", for example, and these might be commands in one's .octaverc file, perhaps?

But maybe what is missing is a copy of factory settings into "default_properties" at the very beginning, i.e., initialization of root_figure.  I'm wondering if that is what reset_default_properties() was intended to do.  But there would also need to be a "reset_properties ()" that copies the "default_properties" into "xproperties" which is called when someone creates a new plot or uses "reset".

So that would give the user the ability to create his or her own default properties in .octaverc thereby overriding the factory defaults.

Should the following maybe be declared static?


private:
  property_list default_properties;


so that the default_properties do not change from new figure to new figure?  (I realize you might not know how the answer to that.)  Otherwise when a new figure is created there is no option but to get "default_properties" from the factory defaults.  That doesn't save much work though because then the user would have to do "set(gcf, "defaultXYZ", xyzval)" after every new figure, in which case one might as well do "set(gcf, "XYZ", xyzval)" after every new figure.

This was probably some loose end that was overlooked in some huge changeset.

Dan Sebald <sebald>
Thu 16 May 2013 10:18:26 PM UTC, comment #26: 

I think the way that is supposed to work is (for example):


get (0, 'factory')
get (0, 'factoryaxes')
get (0, 'factoryaxeszlim')


John W. Eaton <jwe>
Group administrator
Thu 16 May 2013 09:56:14 PM UTC, comment #25: 

Oh, the factory settings are currently associated with the kit.  So,


>> get (0, 'factory')
ans =

  scalar structure containing the fields:

    factoryaxes__hold_all__ = off
    factoryaxes__modified__ = on
    factoryaxesactivepositionproperty = outerposition
...


Do we want the other graphics objects to display their factory defaults (a subset of the result above)?


Dan Sebald <sebald>
Thu 16 May 2013 09:13:28 PM UTC, comment #24: 

Just noting a more basic incorrect result.  I saw in the code:


  octave_value get (const caseless_str& name) const
  {
    return name.compare ("default")
      ? get_defaults ()
      : (name.compare ("factory")
         ? get_factory_defaults () : rep->get (name));
  }


so I tried these inputs and I'm seeing:


>> hax = gca;
>> get (hax, 'default')
ans =

  scalar structure containing the fields:


>> get (hax, 'factory')
error: base_graphics_object::get_factory_defaults: invalid graphics object


The missing defaults we knew about, but factory defaults isn't right either.

Dan Sebald <sebald>
Thu 16 May 2013 08:04:08 PM UTC, comment #23: 

OK, I see now that the argument is being modified.  Maybe it would be clearer to me if it were writtin in a more functional style instead of modifying the reference.  So pass in the current value (const) along with the factory values (also const), and then return the new value, leaving it up to the caller to reset the default_properties in the graphics object.

Is the current design my fault?  If so, sorry about that.

John W. Eaton <jwe>
Group administrator
Thu 16 May 2013 07:53:31 PM UTC, comment #22: 

I was thinking it was something that simple, but it is slightly more difficult.  The "new_defaults" in the general function:


static void
reset_default_properties (property_list& default_properties)
{
  property_list new_defaults;


can't tell where it is supposed to be getting its settings from.  So I think this needs to be broadened to something like


static void
reset_default_properties (property_list& default_properties, const property_list& factory_defaults)
{


and factory_defaults plays the roll of what new_defaults was supposed to be doing.  That would mean using the contrapositive for the conditional test, i.e., copy all settings onto default_properties except those that are supposed to remain the same.

I'll have a patch in a couple hours.

Dan Sebald <sebald>
Thu 16 May 2013 07:28:24 PM UTC, comment #21: 

Do we need to distinguish between default and factory properties?

The simple fix seems to be to change all of the functions like


void
axes::reset_default_properties (void)
{
  ::reset_default_properties (default_properties);
}


to


void
axes::reset_default_properties (void)
{
  ::reset_default_properties (axes::properties::factory_defaults ());
}


But if we need to be able to reset to current defaults and reset to factory defaults, we'll need separate sets of functions, or a paramter that allows us to choose which action to take.

Also, since these functions are all likely to follow exactly the same pattern, maybe they should be automatically generated by the genprops.awk script.

John W. Eaton <jwe>
Group administrator
Thu 16 May 2013 07:16:15 PM UTC, comment #20: 

OK, I found where those are coming from.  There is a BEGIN_PROPERTIES macro in graphics.in.h.  That should get me somewhere.

Dan Sebald <sebald>
Thu 16 May 2013 07:08:40 PM UTC, comment #19: 


> The description "Remove any defaults set for the handle var{h}."  is also not clear to me.


Yes, I think that is where the confusion is.  There is some subtle interpretation here between the default settings and factory settings that I'm not seeing and isn't quite right in the code.  I think "defaults" means what the graphics object will use without any other specification.  Rik gave this example:


hax = gca;
set (hax, 'linewidth', 2.3);


So the default linewidth for the axis object is now 2.3, but I think a user could go through and tweak individual lines.

Maybe a more accurate way of stating it is "Set defaults back to factory settings".

Dan Sebald <sebald>
Thu 16 May 2013 07:00:50 PM UTC, comment #18: 

OK, I'll make a note of that...

I'm sort of remembering this now.  I can describe what the problem is, but I don't know how to solve it because I don't know where the defaults are supposed to be coming from.  (Maybe that's what "factory_defaults" is supposed to be.)

The problem is that this hunk of code in graphics.cc:


reset_default_properties (property_list& default_properties)
{
  property_list new_defaults;

  for (property_list::plist_map_const_iterator p = default_properties.begin ();
       p != default_properties.end (); p++)
    {


never goes inside the loop because "default_properties" is empty.  I've verified that the for-loop is reached, but nothing happens.  The function makes sense: new_defaults is supposed to be a copy of factory_defaults(?) and then the current settings for figure->"position", figure->"units", figure->"windowstyle", figure->"paperunits", axes->"position" and axes->"units" are copied if they are present because those are supposed to be retained.

However, I can't find where "default_properties", or "new_defaults" for that matter, is being set anywhere.  It seems like there should be a ": default_properties(factory_defaults)" in the constructor and maybe:

property_list new_defaults = factory_defaults;

or something like that.  I'm not sure though because those are supposed to come from the particular graphics engine, or maybe from the base class.

Dan Sebald <sebald>
Thu 16 May 2013 06:43:35 PM UTC, comment #17: 

The intent may have been to allow resetting individual properties.  Maybe it would not be hard to do that.

The description "Remove any defaults set for the handle @var{h}."  is also not clear to me.

Matlab has a reset function that resets most properties for the handle H to their "factory" defaults.  If that is not what Octave's reset function is doing, we should probably fix it, since that is the behavior that most people will expect.

John W. Eaton <jwe>
Group administrator
Thu 16 May 2013 06:24:52 PM UTC, comment #16: 

I'm looking at this one again to see if we can close this one out.  The first thing I notice is this inconsistency in the documentation:


>> help reset
'reset' is a built-in function from the file libinterp/interpfcn/graphics.cc

 -- Built-in Function:  reset (H, PROPERTY)


has a second input, but second inputs aren't allowed in graphics.cc:


  int nargin = args.length ();

  if (nargin != 1)
    print_usage ();
  else
    {


What is the case here?  Just a typo and "PROPERTY" should be removed?  Or more code needs to be added to graphics.cc to handle some missing behavior?

Dan Sebald <sebald>
Mon 10 Sep 2012 07:03:29 PM UTC, comment #15: 

I believe the problem is on the "set" side of things, not the "reset" side.  I've followed the code that does the reset and it appears to be run, but there isn't anything in the list of defaults.  I've looked at the property_list "set" for the defaults, and that doesn't appear to happen.  So whatever mechanism existed before for setting the defaults isn't happening.

Dan Sebald <sebald>
Mon 10 Sep 2012 04:58:40 PM UTC, comment #14: 

Yes, I see the same behavior. 

The issue is now with the last plotyy demo.

Example code:


close all
demo ("plotyy", 4)
sombrero
## At this point there is a messed up figure


Maybe also check bug #35511 which indicates that the reset function on a graphics handle is broken.

Rik <rik5>
Group administrator
Mon 10 Sep 2012 04:20:13 PM UTC, comment #13: 

I'm reopening this bug report becuase it doesn't seem to be comppletely fixed yet.

I no longer see an error, and the test


demo ("plotyy", 1);
sombrero


does appear to work.  However, if I do

+vertabim+
demo plotyy
-verbatim-

and step through all the demos, then do


sombrero


I see the attached figure, not the sombrero I was expecting.  So it seems that plotyy is still seting some properties that are not being cleared before creating the sombrero plot.  Does anyone else see this behavior?


John W. Eaton <jwe>
Group administrator
Sun 09 Sep 2012 07:14:33 PM UTC, comment #12: 

Opps!

My bad. I attributed Dan's patch to Philip

Ben Abbott <bpabbott>
Group Member
Sun 09 Sep 2012 06:53:24 PM UTC, comment #11: 

Thanks for fixing, Ben & Dan.
But why is the patch attributed to me? I only raked up the problem...

Philip Nienhuis <philipnienhuis>
Group Member
Sat 08 Sep 2012 06:57:59 PM UTC, comment #10: 
Ben Abbott <bpabbott>
Group Member
Sat 08 Sep 2012 01:59:03 AM UTC, comment #9: 

Looks good here.  The demo/sombrero combination passes and "test rmappdata" passes all tests.

On the "setappdata()" side of things there are tests for existence, etc.  So, I guess the only thing is whether you want to use the existing "isappdata()" in some way, but I think the patch is more efficient.

Thanks,

Dan

Dan Sebald <sebald>
Sat 08 Sep 2012 01:36:30 AM UTC, comment #8: 

Dan, I found a few short comings in your patch and added/modified tests.

Have I missed anything?

(file #26505)

Ben Abbott <bpabbott>
Group Member
Sat 08 Sep 2012 12:28:40 AM UTC, comment #7: 

I think that the demo creates a figure with multiple handles in a way that exercised a route that hasn't been tested as of yet.  Attached is a patch that fixes a spelling error, _ploty_axes_, and also first checks that the _appdata_ field exists before proceeding.

Thanks for reporting this bug.

(file #26504)

Dan Sebald <sebald>
Fri 07 Sep 2012 07:42:11 PM UTC, comment #6: 

I couldn't reproduce the original, but Comment #1 does cause problems on the latest development code.

octave:1> demo ("plotyy", 1)
...
octave:2> sombrero
error: invalid conversion from real matrix to real scalar
error: _calc_dimensions_: expecting graphics handle as only argument
error: called from:
error:   /usr/local/src/octave/octave/build-nogui-8/../octave/scripts/plot/private/__go_draw_axes__.m at line 55, column 8
error:   /usr/local/src/octave/octave/build-nogui-8/../octave/scripts/plot/private/__go_draw_figure__.m at line 173, column 19
error:   /usr/local/src/octave/octave/build-nogui-8/../octave/scripts/plot/__gnuplot_drawnow__.m at line 86, column 5

That's a start.

Dan Sebald <sebald>
Fri 07 Sep 2012 07:24:34 PM UTC, comment #5: 

My cases have all been while using fltk
and on Fedora (currently fc17).

Michael Godfrey <godfrey>
Group Member
Fri 07 Sep 2012 07:10:45 PM UTC, comment #4: 

Vague or not, it might help to emphasize that I couldn't reproduce the original issue with fltk. (I can only vouch for my own PC's, of course).
Did you encounter your problem with gnuplot, fltk or both?


Philip Nienhuis <philipnienhuis>
Group Member
Fri 07 Sep 2012 06:50:19 PM UTC, comment #3: 

This report does seem vague, but
I have noticed that there are conditions
under which:
octave
plot(xx,yy)
      %% change the size of the plot window
      %% and other plot options
....
close
plot(xx,yy)

results in the second plot(xx,yy) not
appearing the same as the first one.
(Using the current devel system.)

This is also very vague and right now I have
not been able to reproduce this behavior.

But, I suspect that close does not completely
reset all plot options.

If I can isolate a test case I will report it
under this bug #.


Michael Godfrey <godfrey>
Group Member
Fri 07 Sep 2012 07:26:00 AM UTC, comment #2: 

It's so obscure that perhaps priority can be decreased? - after all, nobody has noticed since at least 3.4.3.

Philip Nienhuis <philipnienhuis>
Group Member
Thu 06 Sep 2012 09:03:11 PM UTC, comment #1: 

Wow, that's an obscure bug. 

I can confirm this with 3.6.2 on a Linux platform and it also affects the development code.  You don't need to run all the demos.  This shorter sequence reproduces the bug.


demo ("plotyy", 1)
sombrero


Interestingly, if you insert a "clf" between the demo and sombrero function call then it works.  This seems to suggest that sombrero() is not clearing out the pre-existing figure.

Rik <rik5>
Group administrator
Thu 06 Sep 2012 08:35:17 PM UTC, original submission:  

Reproducible on Mandriva 2010.2,, Mageia 2 and Windows (MinGW, MSVC).
Also errors on Octave-3.6.2_MSVC/MinGW & Octave-3.6.1_MSVC

Works OK on MinGW Octave 3.4.3.

So somewhere in between 3.6.1 and 3.4.3 a regression happened.


octave:1> graphics_toolkit gnuplot
octave:2> demo plotyy
plotyy example 1:
 clf
 x = 0:0.1:2*pi;
 y1 = sin (x);
 y2 = exp (x - 1);
 ax = plotyy (x, y1, x - 1, y2, @plot, @semilogy);
 xlabel ("X");
 ylabel (ax(1), "Axis 1");
 ylabel (ax(2), "Axis 2");
 axes (ax(1))
 text (0.5, 0.5, "Left Axis", ...
       "color", [0 0 1], "horizontalalignment", "center")
 axes (ax(2))
 text (4.5, 80, "Right Axis", ...
       "color", [0 0.5 0], "horizontalalignment", "center")

Press <enter> to continue:
plotyy example 2:
 clf
 x = linspace (-1, 1, 201);
 subplot (2, 2, 1)
 plotyy (x, sin(pi*x), x, 10*cos(pi*x))
 subplot (2, 2, 2)
 surf (peaks (25))
 subplot (2, 2, 3)
 contour (peaks (25))
 subplot (2, 2, 4)
 plotyy (x, 10*sin(2*pi*x), x, cos(2*pi*x))
 axis square

Press <enter> to continue:
plotyy example 3:
 clf
 x = linspace (-1, 1, 201);
 subplot (1, 1, 1);
 hax = plotyy (x, sin(pi*x), x, cos(pi*x));
 ylabel ("Blue and on the Left")
 ylabel (hax(2), "Green and on the Right")
 xlabel ("xlabel")

octave:3> sombrero
error: invalid conversion from real matrix to real scalar
error: __calc_dimensions__: expecting graphics handle as only argument
error: called from:
error:   /home/philip/devel/oct363/../octave-3.6.3/scripts/plot/private/__go_draw_axes__.m at line 55, column 8
error:   /home/philip/devel/oct363/../octave-3.6.3/scripts/plot/private/__go_draw_figure__.m at line 167, column 19
error:   /home/philip/devel/oct363/../octave-3.6.3/scripts/plot/__gnuplot_drawnow__.m at line 86, column 5
octave:4>


With a "close" in between (before "sombrero") all goes well; fltk isn't affected either.


Philip Nienhuis <philipnienhuis>
Group Member

 

(Note: upload size limit is set to 16384 kB, after insertion of the required escape characters.)

Attach Files:
   
   
Comment:
   

Attached Files
file #28113:  matlab.png added by bpabbott (10KiB - image/png)
file #28114:  octave.png added by bpabbott (26KiB - image/png)
file #28111:  octave-graphics_reset-2013may19.patch added by sebald (36KiB - application/octet-stream)
file #28098:  bad_attempt_for_defaults_2013may16.diff added by sebald (2KiB - application/octet-stream)
file #26522:  sombrero-screenshot.png added by jwe (15KiB - image/png)
file #26505:  changeset.patch added by bpabbott (2KiB - application/octet-stream)

 

Depends on the following items: None found

Items that depend on this one: None found

 

Carbon-Copy List
  • -email is unavailable- added by jwe (Updated the item)
  • -email is unavailable- added by bpabbott (Updated the item)
  • -email is unavailable- added by sebald (Posted a comment)
  • -email is unavailable- added by godfrey (Posted a comment)
  • -email is unavailable- added by rik5 (Posted a comment)
  • -email is unavailable- added by philipnienhuis (Submitted the item)
  •  

    There are 0 votes so far. Votes easily highlight which items people would like to see resolved in priority, independently of the priority of the item set by tracker managers.

    Only group members can vote.

     

    Follow 18 latest changes.

    Date Changed by Updated Field Previous Value => Replaced by
    2013-05-20 bpabbott Open/ClosedOpen Closed
    2013-05-20 bpabbott Attached File- Added matlab.png, #28113
        Attached File- Added octave.png, #28114
        StatusNone Works For Me
    2013-05-19 sebald Attached File- Added octave-graphics_reset-2013may19.patch, #28111
    2013-05-17 sebald Attached File- Added bad_attempt_for_defaults_2013may16.diff, #28098
    2013-05-16 bpabbott StatusFixed None
        Assigned tobpabbott None
    2012-09-10 jwe Attached File- Added sombrero-screenshot.png, #26522
        Open/ClosedClosed Open
    2012-09-10 rik5 Open/ClosedOpen Closed
    2012-09-08 bpabbott StatusConfirmed Fixed
        Assigned toNone bpabbott
        Release3.6.3 dev
    2012-09-08 bpabbott Attached File- Added changeset.patch, #26505
    2012-09-08 sebald Attached File- Added octave-plotyyandappdata_2012sep07.patch, #26504
    2012-09-07 rik5 Severity3 - Normal 2 - Minor
    2012-09-06 rik5 StatusNone Confirmed

    Back to the top

    Powered by Savane 3.13-3230.
    Corresponding source code