bugGNU Octave - Bugs: bug #30419, set/get vs setappdata/getappdata...

 
 

bug #30419: set/get vs setappdata/getappdata for custom properties

Submitter:  Ben Abbott <bpabbott>
Submitted:  Mon 12 Jul 2010 09:09:51 AM UTC
   
 
Category:  None Severity:  3 - Normal
Priority:  5 - Normal Item Group:  None
Status:  Fixed Assigned to:  None
Originator Name:  bpabbott Open/Closed:  * Closed
Release:  * dev Operating System:  * Mac OS
Fixed Release:  None Planned Release:  None
* Mandatory Fields

Add a New Comment Rich Markup
   

Jump to the original submission

Wed 03 Nov 2010 07:18:36 AM UTC, comment #6: 

Ok. I've pushed an updated changeset.

    http://hg.savannah.gnu.org/hgweb/octave/rev/1a26199cb212


Ben Abbott <bpabbott>
Group Member
Wed 03 Nov 2010 12:30:20 AM UTC, comment #5: 

The choice between using .m files or c++ is usually done based on required efficiency.  I can't see that these functions are going to be called frequently from the innermost loop of some tight ball of code where performance is crucial.  In this case a .m file implementation is just fine.  Why don't you go ahead and commit this patch.

Rik <rik5>
Group administrator
Sun 29 Aug 2010 07:25:34 PM UTC, comment #4: 

I'm unfamiliar with out calling m-files works from the c++ side. Is there any functional reason to not push this changeset?

Ben Abbott <bpabbott>
Group Member
Thu 15 Jul 2010 10:07:40 PM UTC, comment #3: 

I've attached a proposal for an m-file implementation.

I chose to add a new property, _appdata_, to hold the application data. This has the advantage of avoiding any conflicts with names and is m-file compatible with the Matlab versions.

However, I expect it would be preferred that they be implemented in c++.

I haven't changed the "Assigned to" field, since I'm not up to a c++ implementation. If an m-file implementation is acceptable, then I'll do it.

(file #20988)

Ben Abbott <bpabbott>
Group Member
Tue 13 Jul 2010 12:07:58 AM UTC, comment #2: 

The only conflict I see with having setappdata/getappdata work with the same properties as set/get is with property name conflicts.

Might an "appdata" property, be added to each object, with that property acting like a structure whose fieldnames correspond to the appdata properties? (forgive me if my understanding of  the c++ side sounds garbled, the only c/c++ programming I've done is mimicry)

Another possibility would be to do as you suggest, but have set/getappdata pre/postpend "__" to each property name.

function setappdata (h, name, val)
  addproperty (strcat ("__", name, "__"), h, "any", val);
endfunction

function getappdata (h, name, val)
  get (h, strcat ("__", name, "__"));
endfunction

Regarding relying on the undocumented mechanism Matlab uses, I don't see any reason to copy it. I only mentioned it since is a recent example I found when poking around while on vacation ... I later discovered you'd submitted a changeset.








Ben Abbott <bpabbott>
Group Member
Mon 12 Jul 2010 07:34:55 PM UTC, comment #1: 

Why can't we have Octave style properties added to the base properties and setappdata/getappdata properties as well. For example with the functions

function setappdata (h, name, val)
   addproperty (name, h, "any", val);
endfunction

function x = isappdata (h, name)
  x = any (strcmp (name, fieldnames (getappdata (h))));
endfunction

function rmappdata (h, name)
   ## FIXME Need to write this function in graphics.cc
   delproperty (h, name);
endfunction

and then add the code

DEFUN (getappdata, args, , "there's no help for the wicked")
{
  int nargin = args.length ();
  octave_value retval;

  if (nargin != 1 || nargin != 2)
    print_usage ();
  else
    {
      double h = args(0).double_value ();

      if (! error_state)
        {
          graphics_handle gh = gh_manager::lookup (h);

          if (gh.ok ())
            {
              graphics_object go = gh_manager::get_object (gh);


              if (nargin == 1)
                retval = go.get_properties ().get_dynamic ();
              else
                {
                  std::string name = args(1).string_value ();

                  if (! error_state)
                    retval = go.get_properties ().get_dynamic (name);
                }
            }
        }
    }
}

to src/graphics.cc. In this manner you could keep the old behavior and be compatible with matlab. If you want to get fancy you could do it all in graphics.cc, or at least the isappdata function as this could be more efficient as a DEFUN in graphics.cc.

For the fact the the appdata of the axes holds the data for the matlab line color, style and the hold style, this is undocumented and we shouldn't rely on it and don't need to implement it in the same fashion. Unless you're telling me its common knowledge and many people in the matlab world rely on this behavior..

As for the COM versions of addproperty, I say tough luck.

D.

David Bateman <dbateman>
Group Member
Mon 12 Jul 2010 09:09:51 AM UTC, original submission:  

Matlab doesn't allow users to create new properties for its objects in the way Octave does. Matlab's "addproperty" serves a different purpose than Octaves'. The Matlab variant adds a custom property to a COM object, which are available on Microsoft Windows systems only.

http://www.mathworks.com/access/helpdesk/help/techdoc/ref/addproperty.html
http://www.mathworks.com/access/helpdesk/help/techdoc/matlab_external/bqdwu3j.html

Instead Matlab uses setappdata(), getappdata(), isappdata() to allow custom properties to be managed. The first two, work in a manner consistent with set() and get(), except that they are case sensitive. The latter works as ...

isappdata = @(h, name) any (strcmp (name, fieldnames (getappdata (h))));

A simple example of this is found in the mechanism by which Matlab manages the hold state for plots.

>> clf
>> plot (1:10)
>> p = getappdata (gca)


p =

        PlotColorIndex: 2
    PlotLineStyleIndex: 1

>> hold all
>> p = getappdata (gca)


p =

        PlotColorIndex: 2
    PlotLineStyleIndex: 1
         PlotHoldStyle: 1

>> hold off
>> p = getappdata (gca)


p =

        PlotColorIndex: 2
    PlotLineStyleIndex: 1
         PlotHoldStyle: 0

>> hold on
>> p = getappdata (gca)


p =

        PlotColorIndex: 2
    PlotLineStyleIndex: 1
         PlotHoldStyle: 0

Regarding this example, I'd have responded to the "hold all" discussion but on the tracker, but was out of email contact.

Ben Abbott <bpabbott>
Group Member

 

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

Attach Files:
   
   
Comment:
   

Attached Files
file #20988:  changeset.patch added by bpabbott (9KiB - application/octet-stream - proposed changeset for an m-file implementation)

 

Depends on the following items: None found

Items that depend on this one: None found

 

Carbon-Copy List
  • -email is unavailable- added by rik5 (Posted a comment)
  • -email is unavailable- added by dbateman (Posted a comment)
  • -email is unavailable- added by bpabbott (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 4 latest changes.

    Date Changed by Updated Field Previous Value => Replaced by
    2010-11-04 rik5 StatusReady For Test Fixed
        Open/ClosedOpen Closed
    2010-07-15 bpabbott Attached File- Added changeset.patch, #20988
        StatusNone Ready For Test

    Back to the top

    Powered by Savane 3.13-02a9.
    Corresponding source code