[Octave-bug-tracker] [bug #45833] support load/save of classdef objects

[Thomas]

Follow-up Comment #7, bug #45833 (group octave):

I added functionality for loading and saving classdef files in the native
Octave text format. I wanted some feedback on my patch before I go ahead and
finish the rest of the functionality.

I've written in the patch notes about how these functions should behave, but
here's some additional comments:
* The patch supports custom loadobj and saveobj methods that can be defined
within the classdef file.
* Saving classdef objects to a file needs to encode additional metadata to the
file. The only piece of metadata I've implemented so far is when the return
value of a custom 'saveobj' method is not a struct or an object, so that the
corresponding loading process must be customized with a 'loadobj' method.
Credit to @foreverallama (tag on Discourse) for discovering through
reverse-engineering that  MAT files support a similar flag. More metadata will
probably be needed to add support for handle classes, which currently are
saved and loaded as value classes.
* In that vein, part of the reason why I decided to implement the native file
format loading and saving is because we need a way to encode the metadata. If
we depreciate the native Octave format and go only with the MAT format, then
we have to rely on reverse-engineering undocumented flags in the binary files,
when it is not super clear to understand what each one of them do. I'm not
opposed to converging on only one open file format, but it needs to be
precisely defined and documented (and a bonus if it can be parsed by external
tools).
* loadobj and saveobj in ov-classdef.cc are meant to be used independent of
the file format, so that anyone implementing other file formats can just
simply call these methods without having to re-implement large parts of the
load and save logic.
* The type listed in the save file is the name of the classdef being saved,
NOT the type 'object'. So when read_text_data in ls-oct-text.cc pulls the
type, it needs to look into the load path for all the files saved, and finds
the corresponding classdef with the same name. 
* When the loader finds the appropriate classdef file, if property names do
not match, MATLAB's default loading behaviour is to load the ones that do and
silently ignore those that don't. We can issue a warning here, I think it
would be a good idea on our end.
* If a classdef file is not found, MATLAB tries to load the file format as a
uint32; I think this dumps the binary contents of the entire file? In this
patch, loading fails if an appropriate classdef is not found.
* Private, protected and hidden properties are saved and loaded just like
public ones. Curiously enough in MATLAB, you can instantiate a class like
this,

classdef foo
  properties (Access = public)
    a 
  end
  properties (Access = private)
    b
  end
end

set 'a' and 'b' to be defined values, save the object, and then change the
properties like such, swapping the access attributes

classdef foo
  properties (Access = public)
    b
  end
  properties (Access = private)
    a
  end
end

and then load the object; the values of 'a' and 'b' stay the same, despite
their access attributes changing.
* The 'Transient' attribute in Octave is not supported (can't be parsed), but
in MATLAB, this essentially marks the properties that shouldn't be saved.
* Because the default behaviour of loading an object will default initialize
any variables in the classdef that are not loaded in, I had to edit the
construct method in cdef-class.h/.cc to accept an optional default argument
that determines whether the construction of an object must call the
constructor. This might not be acceptable as a change, but then there has to
be some other kind of workaround.
* The 'ConstructOnLoad' attribute is not supported in this patch yet, but this
shouldn't be a big change.
* Is there any way for the caller of a function to silence a warning that
occurs within the called function? 
* Lastly, I have written a small test suite for this change. If anyone has the
time to test if the basic functionality works on their specific classes and
packages, I would be very grateful to hear back about that.

(file #57358)

    _______________________________________________________

Additional Item Attachment:

File name: load_save_patch.diff           Size: 21KiB
    <https://file.savannah.gnu.org/file/load_save_patch.diff?file_id=57358>


    AGPL NOTICE

These attachments are served by Savane. You can download the corresponding
source code of Savane at
https://savannah.gnu.org/source/savane-e6e5367e43c4f3277d32091b77b783b4fe8d5c20.tar.gz


    _______________________________________________________

Reply to this item at:

  <https://savannah.gnu.org/bugs/?45833>

_______________________________________________
Message sent via Savannah
https://savannah.gnu.org/

signature.asc
Description: PGP signature