bugGNU Octave - Bugs: bug #45833, support load/save of classdef...

 
 

bug #45833: support load/save of classdef objects

Submitter:  Mike Miller <mtmiller>
Submitted:  Thu 27 Aug 2015 12:09:04 PM UTC
   
 
Category:  Classdef Severity:  1 - Wish
Priority:  5 - Normal Item Group:  Feature Request
Status:  In Progress Assigned to:  None
Originator Name:  Open/Closed:  * Open
Release:  * dev Release: 
Operating System:  * Any Fixed Release:  None
Planned Release:  None
* Mandatory Fields

Add a New Comment Rich Markup
   

Jump to the original submission

Sun 13 Jul 2025 08:48:04 PM UTC, comment #31: 

@Philip, yes there is a parallel effort by @foreverallama to load and save .mat files fully compatible with MATLAB. I have no thoughts on whether those patches should be submitted in this bug report or not, it's all the same to me.

For the record, there's a number of unsupported features still remaining to implement for loading and saving in the text format, so I personally believe this report should stay open.

Thomas <kolmanthomas>
Sun 13 Jul 2025 11:08:34 AM UTC, comment #30: 

Just FTR:

Is the final aim to not only be able to save/load classdef objects in Octave, but also to be able to:

  • load .mat-files containing classdef objects generated by Matlab and
  • save classdef objects in Octave to .mat files to be read later on by Matlab?

AFAIK Matlab doesn't support Octave's uncompressed "-text" format.

Philip Nienhuis <philipnienhuis>
Group Member
Sat 12 Jul 2025 06:29:16 PM UTC, comment #29: 

Please take a look at bug #67311 about a massive slowdown in performance after this changeset.


changeset:   34705:38180b218448
user:        Thomas Kolman <realkolmanthomas@gmail.com>
date:        Tue Jul 01 15:28:49 2025 -0400
summary:     Load and save classdef objects of value classes in "-text" format (bug #45833).


Rik <rik5>
Group administrator
Sat 12 Jul 2025 03:08:35 PM UTC, comment #28: 

I pushed part3 and part4 to the default branch:
https://hg.savannah.gnu.org/hgweb/octave/rev/38180b218448
https://hg.savannah.gnu.org/hgweb/octave/rev/2b1e9d51ed45

I hope that doesn't interfere with a decision about how to deal with handle class objects. But we'll probably need to take some major decisions in that respect anyway. Worst case we can (partly?) revert some of the changes we applied so far.

But with that, it should now be possible to save and load value class objects in "-text" format.
That's a big achievement. Thank you to everyone involved!

Markus Mützel <mmuetzel>
Group administrator
Fri 11 Jul 2025 11:33:40 PM UTC, comment #27: 

Sorry for my tardy replies. I was debugging a fairly painful MacOS issue when it came to building Octave that was related to some external packages from a Homebrew update... Anyways, that's now fixed, I've tested your patches, and everything that is known to work seems to work well!

> Afaict, the `regular_class_with_constructor` classdef doesn't actually have the `(ConstructOnLoad)` attribute.
> Am I missing something?


> Also in the currently disabled test for "Transient" properties, should the comment say "Transient property should not be saved and loaded"?


No, you are correct and not missing anything, these were mistakes on my part.

> The attached updated version of the part3 patch addresses the remaining questions about it:
> It adds a new classdef file with a class that has the "ConstructOnLoad" attribute.
> It enables the (currently failing) test for the "Transient" property attribute and negates the comment.
> It tags all tests that are currently failing with this bug report number. If we prefer to open separate reports for (some or all of) these issues, we can still do that.


You are totally correct to apply fixes on all these points. I don't know what I was testing before, but the Transient attribute does seem able to be parsed. That is why I disabled the test btw, I was expecting a parse error for some reason...

> The attached patch is a slightly cleaned up version of the part4 (loading N-d arrays) that applies on top of the latest version of part3.
> It also adds a (currently failing) test with a 2-d array of classdef objects that references bug #65179 (where reshaping classdef arrays could be implemented).


> If no one opposes, that patch could probably also be applied.


This is really cool, I tried it and it seems to work for 1xN arrays at least, including the statistics package example from Dmitri. I certainly don't oppose it.

> Regarding the handling of handle classes: If I understand correctly, @foreverallama already has an implementation for loading handle classes (including ones with circular self-references).
> Maybe, we could try and share that idea for all file formats.


I will have a look at this. I don't necessary want to start a parallel third conversation, so I'll reply more in-depth on Discourse/GitHub when I've thought more about it. But I suspect, like my issue with custom loadobj return types, that it would require a pretty significant refactoring of the saving/loading chain of events happening as of right now. But perhaps that might end up being a boon, because I've also noticed that the loading and saving of classdefs seems awfully slow.

Thomas <kolmanthomas>
Fri 11 Jul 2025 01:59:25 PM UTC, comment #26: 

The attached patch is a slightly cleaned up version of the part4 (loading N-d arrays) that applies on top of the latest version of part3.
It also adds a (currently failing) test with a 2-d array of classdef objects that references bug #65179 (where reshaping classdef arrays could be implemented).

If no one opposes, that patch could probably also be applied.


(file #57395)

Markus Mützel <mmuetzel>
Group administrator
Fri 11 Jul 2025 11:43:50 AM UTC, comment #25: 

The attached updated version of the part3 patch addresses the remaining questions about it:
It adds a new classdef file with a class that has the "ConstructOnLoad" attribute.
It enables the (currently failing) test for the "Transient" property attribute and negates the comment.
It tags all tests that are currently failing with this bug report number. If we prefer to open separate reports for (some or all of) these issues, we can still do that.


I'd like to push that patch to the repository if no one opposes.



(file #57394)

Markus Mützel <mmuetzel>
Group administrator
Fri 11 Jul 2025 08:44:59 AM UTC, comment #24: 

I agree with you that it is worthwhile to implement loading and saving classdef objects in the '-text' format. I plan on pushing your part3 when the remaining open points have been resolved.
Those are mostly regarding the test cases.
Apart from the two questions from comment #23: Is there a reason why the xtest for the 'Transient' attribute is currently disabled?
Should we tag the current xtests with this bug number? Or do you prefer separate reports for them?


Coming back to the "part4" patch:
If I understand correctly, there is currently no way of creating arbitrarily shaped N-D arrays of classdef objects without creating temporary objects that use the default constructor in general. That would probably fail if the class constructor requires input arguments.

The only shape(s) that do not require creating temporary (default constructed) objects are row or column vectors.

Maybe, we could start with loading every N-D array as a vector (potentially with warning) until we have a working reshape-operation on classdef-object arrays.
What do you think?


Regarding the handling of handle classes: If I understand correctly, @foreverallama already has an implementation for loading handle classes (including ones with circular self-references).
Maybe, we could try and share that idea for all file formats.


Coming back to the issue you had with the 'Transient' attribute for the properties blocks in Octave: I'm having trouble reproducing that locally. Could you please share an example which doesn't work for you?

Markus Mützel <mmuetzel>
Group administrator
Thu 10 Jul 2025 07:48:05 AM UTC, comment #23: 

I split the part2 into two parts again and pushed them with some additional comments to the repository:
https://hg.savannah.gnu.org/hgweb/octave/rev/f3b7f66834c1
https://hg.savannah.gnu.org/hgweb/octave/rev/eb44a98749ad

Regarding the tests that are added in part3, there is the following xtest:

%% Constructor, ConstructOnLoad = true, no loadobj/saveobj
%!xtest
%! obj = regular_class_with_constructor;
%! obj.a = [];
%! savefile = tempname ();
%! save ('-text', savefile, 'obj');
%! unwind_protect
%!   clear obj;
%!   load (savefile);
%!   assert (obj.a, 1);
%! unwind_protect_cleanup
%!   delete (savefile);
%! end_unwind_protect


Afaict, the `regular_class_with_constructor` classdef doesn't actually have the `(ConstructOnLoad)` attribute.
Am I missing something?

Also in the currently disabled test for "Transient" properties, should the comment say "Transient property should not be saved and loaded"?

I'm attaching an updated version of the part3 that is rebased on the current head of the repository.


(file #57391)

Markus Mützel <mmuetzel>
Group administrator
Wed 09 Jul 2025 10:43:30 PM UTC, comment #22: 

The attached part4 is a very early version that adds support for loading back classdef arrays.

It doesn't yet restore the array in the original shape. But other than that, I think it should be working.


(file #57389)

Markus Mützel <mmuetzel>
Group administrator
Wed 09 Jul 2025 08:58:59 PM UTC, comment #21: 


> 4. I'll try to find a way to detect and save object arrays. I think you can make object arrays, but huge amounts of functionality simply don't work.


I believe that saving object arrays already kind of works.
Creating an object array when loading the file is tricky though...

Hopefully, we'll only need to implement that once and can reuse it for all file formats...

Markus Mützel <mmuetzel>
Group administrator
Wed 09 Jul 2025 08:54:51 PM UTC, comment #20: 

Apparently, there is a way to work around bug #65179.
From `fitdist.m` in the statistics package:

        pd = BurrDistribution.fit ...
             (x(g==1), alpha, censor(g==1), freq(g==1), options);
        for i = 2:groups
          pd(i) = BurrDistribution.fit ...
                  (x(g==i), alpha, censor(g==i), freq(g==i), options);



I'm attaching updated versions of the patches which contain the following changes (apart from minor style cleanups including the renaming that you mentioned):

part2:
Do not emit a warning when converting the object to a structure in the saving process.
part3:
Some initial preparations to support classdef arrays. (Doesn't work yet).

I was hoping that adding support for arrays would be more straight-forward. But the only thing that I could think about is pretty convoluted. (And it's getting late here.)

I'd guess that part2 is pretty close to being ready to be pushed.
Does it look ok to you?


(file #57387, file #57388)

Markus Mützel <mmuetzel>
Group administrator
Wed 09 Jul 2025 06:41:54 PM UTC, comment #19: 

@Dimitri, I am working off dev. Object arrays are not supported right now, so saving each object into a cell array is the best workaround I can offer right now. I will have to provide some warning if this situation is encountered, though.

@Markus,

Firstly, thank you for your detailed feedback and for linting the code. I genuinely appreciate the length you went to for this.

> In ls-oct-text.cc:379, you are writing the class name for classdef objects at the line with "# type:". Don't we need that also if `mark_global` is true?


You are correct about `mark_global`, absolutely.

> I moved them up one level in the folder hierarchy and added them to the build system (so that they will be included in a release tarball). Additionally, I changed them to write to the tmp folder (where we should have write permission always) and to clean up the saved files.


The test changes look very good to me, thank you for cleaning it up.

> I see a couple of warnings if I run these tests manually (instead of as part of `make check`). Is that expected?


The warning is from a call to `map_value()` in octave-classdef, which in turn calls the `map_value()` method in cdef-object, where the warning is automatically triggered. I think we can get around this by adding a default parameter to the `map_value()` method (much like we did for default initialization of objects) to silence the warning.

> After consulting with @foreverallama, I split your patch into three parts:
> 1. Support skipping the constructor of classdef objects.
> 2. Add support for loadobj and saveobj methods of classdef objects.
> 3. Load and save classdef objects of value classes in "-text" format.


I agree with how you split up the patches, it's a good compromise between merging some changes and working out the kinks in the rest. I think you can push (2) as well to dev, since they're private methods to octave-classdef and shouldn't impact anything else yet, but that's personal preference.

As for patch (3), I know there's been some discussion with deprecating formats. My intention was to get the ball rolling on loading/saving of classdef objects in any format, the native text format was just a convenient vehicle to explore the loading and saving process itself. I'm not against just finishing it up, it's a fairly simple format, but I saw some threads on Discourse (can't seem to find it now...) that promoted the idea of a superset of MATLAB HDF5 based .mat files, that sounds like it could consolidate a bunch of formats into one. However, I know nothing about HDF5, so it would take some time to work this out. I just like the thought of being able to inspect the files with external parsing tools like 'h5dump'. My preference would be to work out the rough edges, merge the basic functionality of saving in the native text format (so that a user could save all their workspace variables without any surprises), and continue to explore the superset option down the line. Just food for thought.

A few next steps:

1. In bug45833-part2-classdef-loadobj-saveobj.patch, I accidentally wrote in octave-classdef.h


octave_map saveobj(bool& saveobj_defined)


but in order to match the .cc file, it should've been


octave_map saveobj(bool& custom_saveobj_ret_type)


2. I've posted on Discourse about a design block I don't know how to overcome. I think this would require some fairly invasive changes to how all variables are loaded and saved, but if anyone sees a much easier way to fix it, I'd be delighted to hear it. Here's what I posted:

> As an aside, when it comes to ‘loadobj’. Normally, when you save any other datatype (numeric type, struct, cell), you expect that upon load, you’ll get the same datatype back; I suspect this is what drove the design decision to have the type have all the load_* methods inside each octave type, instead of a separate interface. However this isn’t true of classdef objects, because you can return any type from a custom-defined loadobj method. So there’s a problem in the sense that ‘load_*’ is called on an already-instantiated object that doesn’t actually need to be instantiated, and in fact a totally separate datatype should have been created. It’s a bit of a design roadblock in the load/save mechanism that I ignored so far, but I think it needs to be addressed.


> Here’s an example classdef of a custom loadobj method in MATLAB (R2024b):



classdef Foo
    properties
        x;
    end

    methods
        function retval = saveobj(this)
            retval.a = "abcde";
        end
    end

    methods (Static)
        function retval = loadobj(s)
            retval = s.a;
        end
    end
end


> and here’s the behaviour, as you can see, it returns a string



>> f = Foo;
>> save("obj");
>> clear all;
>> load("obj");
>> f

f =

    "abcde"

>> class(f)

ans =

    'string'


> so the saving type is still an object (because it has to maintain a reference to ‘loadobj’), but it’s saving a struct that gets unpacked in loadobj and loads as a string. Quite a mess.


3. I think we can be a lot better than MATLAB about providing some warnings for unexpected loading and saving behaviour. There's some genuinely tricky behaviour that gets silently passed over, like if 'loadobj' is not static, or if some property names don't match between the save file and the classdef. I'll work on adding this, and adding warnings for unsupported operations (like saving and loading handle classes).

4. I'll try to find a way to detect and save object arrays. I think you can make object arrays, but huge amounts of functionality simply don't work.

5. I have avoided taking a look at parsing code, but if anyone can implement the 'Transient' parsing functionality, that'd be fantastic. Otherwise I'll do it, but it's lower priority for me.

6. I'll look at implementing saving and loading for handle classes as well.

Thomas <kolmanthomas>
Wed 09 Jul 2025 04:41:10 PM UTC, comment #18: 

Regarding the issues with the 1x2 BurrDistribution object: That might be bug #65179.

I don't know how the statistics packages worked around that limitation. But that might be causing issues here...

Markus Mützel <mmuetzel>
Group administrator
Wed 09 Jul 2025 03:35:05 PM UTC, comment #17: 

After consulting with @foreverallama, I split your patch into three parts:
1. Support skipping the constructor of classdef objects.
2. Add support for loadobj and saveobj methods of classdef objects.
3. Load and save classdef objects of value classes in "-text" format.

I went ahead and pushed the first part to the default branch:
https://hg.savannah.gnu.org/hgweb/octave/rev/b3e53f9a6050

That should remove the overlap between your patch and the changes on their branch. With that, you guys should be able to work more independently on the respective implementation for the mat5 or text format.

The attached patches are the second and third part of your original patch after the split. (No "part1" because I pushed that to the repository already.)


(file #57380, file #57381)

Markus Mützel <mmuetzel>
Group administrator
Mon 07 Jul 2025 05:24:23 PM UTC, comment #16: 

Thank you again for these valuable contributions.

Just some initial feedback. Not completely reviewed yet.

In ls-oct-text.cc:379, you are writing the class name for classdef objects at the line with "# type:". Don't we need that also if `mark_global` is true?

I've changed that in the attached patch where I also slightly adapted to Octave's coding style (mostly whitespaces) and slightly changed the commit message (using command style - e.g., "Add" instead of "Added" or "Adds"; breaking lines before 80 characters; bug number in parenthesis at the end of the first line).

I like that you added tests.
I moved them up one level in the folder hierarchy and added them to the build system (so that they will be included in a release tarball). Additionally, I changed them to write to the tmp folder (where we should have write permission always) and to clean up the saved files.
I wasn't exactly sure what was intended with the xtest for handle  classes. The test case as it was couldn't possibly pass afaict. So, I changed it to do something that I think might be useful. Please, check if that was your intension with that test.
I also changed the test files to use "specific ends" and slightly adapted the coding style (mostly white-spaces and parenthesis). I also fixed a typo in transient_property_class.m ("properties" instead of "property").

In general, we prefer to refer to a bug report for failing tests instead of "%!xtest" (e.g., "%!test <45833>" if the test should be passing when this report will be closed.)

I see a couple of warnings if I run these tests manually (instead of as part of `make check`). Is that expected?

warning: struct: converting a classdef object into a struct overrides the access restrictions defined for properties. All properties are returned, including private and protected ones.



Do those changes look ok to you? If they do, would you prefer if we pushed them now and follow up with additional patches to fix issues or add missing parts? Or would you prefer to continue with this patch before we push it to the repository?


(file #57373)

Markus Mützel <mmuetzel>
Group administrator
Fri 04 Jul 2025 10:08:22 PM UTC, comment #15: 

I see some fairly large offsets when applying the patch. Are you working off dev (octave 11) or stable (10.2)?

Dmitri.
--

Dmitri A. Sergatskov <dasergatskov>
Fri 04 Jul 2025 10:02:18 PM UTC, comment #14: 

I am getting the same error.

octave:1> pkg load statistics
octave:2> load d2.txt
octave:3> whos
Variables visible from the current scope:

variables in scope: top scope

  Attr   Name        Size                     Bytes  Class
  ====   ====        ====                     =====  =====
         pd          1x1                          0  BurrDistribution
         x1        100x1                        800  double
         x2        100x1                        800  double

Total is 201 elements using 1600 bytes

octave:4> pd(1)
ans =
error: sprintf: wrong type argument 'cell'
octave:5>


Dmitri.
--

Dmitri A. Sergatskov <dasergatskov>
Fri 04 Jul 2025 09:21:19 PM UTC, comment #13: 

Firstly, thanks for trying it out Dmitri and Colin. So, as far as I'm aware, arrays of classdef objects are not supported (or at least very buggy), see bug#65179. I'm not sure what the statistics package is doing here to return an array of them, I'll try to take a deeper look at it in a few days or so. As an aside, implementing arrays of objects is also something I want to look at, since Chebfun (the software I want to port from MATLAB) is heavily reliant on arrays of objects.

NOTE: I found an indexing error in the previous patch thanks to this example, having to do with loading the correct values for the properties, so I've attached an amended patch with a single line change in line 250 of the diff. You need the amended patch to make this example work.

If you convert the array into a cell array, like so, you should be able to save it


octave> pkg load statistics
octave> x1 = burrrnd (1, 2, 1, 100, 1);
octave> x2 = burrrnd (1, 0.5, 2, 100, 1);
octave> pd = fitdist ([x1; x2], "burr", "By", [ones(100,1); 2*ones(100,1)]);
octave> pd(1)
ans =
  BurrDistribution

  normal distribution
    alpha = 0.729916   [0.627656, 0.848836]
        c =  2.72647   [2.30292, 3.22792]
        k = 0.592502   [0.317158, 1.10689]

octave> pd(2)
ans =
  BurrDistribution

  normal distribution
    alpha = 2.74316   [1.68969, 4.45344]
        c =  0.5147   [0.440233, 0.601763]
        k = 3.26299   [0.714614, 14.8991]

octave> c = {pd(1), pd(2)};
octave> clear pd;
octave> whos
Variables visible from the current scope:

variables in scope: top scope

  Attr   Name        Size                     Bytes  Class
  ====   ====        ====                     =====  =====
         c           1x2                          0  cell
         x1        100x1                        800  double
         x2        100x1                        800  double

Total is 202 elements using 1600 bytes

octave> save -text "d2.txt"

then for loading,

octave> pkg load statistics
octave> load "d2.txt"
octave> whos
Variables visible from the current scope:

variables in scope: top scope

  Attr   Name        Size                     Bytes  Class
  ====   ====        ====                     =====  =====
         c           1x2                          0  cell
         x1        100x1                        800  double
         x2        100x1                        800  double

Total is 202 elements using 1600 bytes

octave> c{1}
ans =
  BurrDistribution

  normal distribution
    alpha = 0.729916   [0.627656, 0.848836]
        c =  2.72647   [2.30292, 3.22792]
        k = 0.592502   [0.317158, 1.10689]

octave> c{2}
ans =
  BurrDistribution

  normal distribution
    alpha = 2.74316   [1.68969, 4.45344]
        c =  0.5147   [0.440233, 0.601763]
        k = 3.26299   [0.714614, 14.8991]


Colin, I'll think of a way to provide a better error message, sure thing.

(file #57363)

Thomas <kolmanthomas>
Thu 03 Jul 2025 10:29:30 PM UTC, comment #12: 

Just a note that for me the tests all pass on the default branch with the patch.

But I also see the same results as Dmitri.

In the case where the classdef isn't available (e.g., statistics pkg not loaded) can we tweak the error message?


load d2.txt
error: octave_base_value::load_ascii(): wrong type argument '<unknown type>'

I'd rather see something like:

class "BurrDistribution" isn't defined.  Perhaps you need to load a package before loading this file?


Colin Macdonald <cbm>
Thu 03 Jul 2025 05:37:37 PM UTC, comment #11: 

Also:

octave:4> load d2.txt
octave:5> pd(1)
ans =
error: sprintf: wrong type argument 'cell'
octave:6> pd
pd =
error: sprintf: wrong type argument 'cell'


Expectations:

octave:6> pd
pd =
1x2 BurrDistribution array
octave:7> pd(1)
ans =
  BurrDistribution

  normal distribution
    alpha = 0.981813   [0.839183, 1.14868]
        c =  2.23293   [1.89373, 2.63289]
        k = 0.917268   [0.440158, 1.91154]

octave:8> pd(2)
ans =
  BurrDistribution

  normal distribution
    alpha = 0.710769   [0.411147, 1.22874]
        c = 0.514213   [0.437374, 0.60455]
        k =  1.81204   [0.815936, 4.0242]

octave:9> pd(3)
error: pd(3): out of bound 2 (dimensions are 1x2)


Dmitri.
--

Dmitri A. Sergatskov <dasergatskov>
Thu 03 Jul 2025 05:28:54 PM UTC, comment #10: 

Oh yes, it kind of works:

octave:1> pkg load statistics
octave:2> load d2.txt
octave:3> whos
Variables visible from the current scope:

variables in scope: top scope

  Attr   Name        Size                     Bytes  Class
  ====   ====        ====                     =====  =====
         pd          1x1                          0  BurrDistribution
         x1        100x1                        800  double
         x2        100x1                        800  double

Total is 201 elements using 1600 bytes


Note, the size of "pd" is different.

Dmitri.
--




Dmitri A. Sergatskov <dasergatskov>
Thu 03 Jul 2025 05:22:43 PM UTC, comment #9: 

You would need to load the statistics package to make sure the classdef file can be seen in the load path, so

octave:1> pkg load statistics
octave:2> load "d2.txt"

should load without error.

Thomas <kolmanthomas>
Thu 03 Jul 2025 04:47:23 PM UTC, comment #8: 

See also
https://savannah.gnu.org/bugs/?66885
Doing


octave:2> pkg load statistics
octave:3> x1 = burrrnd (1, 2, 1, 100, 1);
octave:4> x2 = burrrnd (1, 0.5, 2, 100, 1);
octave:5> pd = fitdist ([x1; x2], "burr", "By", [ones(100,1); 2*ones(100,1)]);
octave:6> save -text d2.txt
warning: struct: converting a classdef object into a struct overrides the access restrictions defined for properties. All properties are returned, including private and protected ones.
octave:7> whos
Variables visible from the current scope:

variables in scope: top scope

  Attr   Name        Size                     Bytes  Class
  ====   ====        ====                     =====  =====
         pd          1x2                          0  BurrDistribution
         x1        100x1                        800  double
         x2        100x1                        800  double

Total is 202 elements using 1600 bytes

Restart octave:

octave:1> load "d2.txt"
error: octave_base_value::load_ascii(): wrong type argument '<unknown type>'


Dmitri.
--


Dmitri A. Sergatskov <dasergatskov>
Wed 02 Jul 2025 04:41:51 PM UTC, comment #7: 

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)

Thomas <kolmanthomas>
Fri 28 Aug 2015 11:06:01 AM UTC, comment #6: 

Parallel execution with the 'parallel' package, functions 'parcellfun' (local) or e.g. 'netcellfun' (cluster), relies on the values of single variables being transfered with Octaves save/load methods (currently 'save_binary_data()' is called). It must be recognized if sending (with a save method) a value failed, otherwise the reading process will just hang.

Since there may exist types derived from octave_base_value which can't be saved (as e.g. types provided by some packages), it is not feasible for the 'parallel' package to keep track which types can be saved and which can't. So if you should change the error into a warning, could you please provide an option of these methods to request the previous behaviour, i.e. an error?

Olaf

Olaf Till <i7tiol>
Group Member
Thu 27 Aug 2015 09:37:58 PM UTC, comment #5: 

That's probably a good thing to do so that any strange octave_value object, like a 'magic colon' or 'lazy index', won't cause the save process to abort.

It still might be worthwhile to override the base save_ascii, load_ascii, save_binary, load_binary in ov-classdef.cc because then you could tailor the warning message.  Instead of


warning: octave_base_value::save_ascii(): wrong type argument 'object'


you could have something like that done for java objects


save: unable to save java objects, skipping


or for onCleanup objects


warning: onCleanup: load and save not supported



Rik <rik5>
Group administrator
Thu 27 Aug 2015 08:52:48 PM UTC, comment #4: 

Those functions already exist in the octave_base_value class.  They currently call gripe_wrong_type_arg.  That function already accepts an optional argument (default value = true) to say whether to issue an error or warning.  So this behavior should be fairly trivial to change.

John W. Eaton <jwe>
Group administrator
Thu 27 Aug 2015 08:35:31 PM UTC, comment #3: 

@jwe: It definitely would be a good idea to have placeholder save functions that issue a warning, but allow any other variables to be saved.  I did that for java objects recently in this changeset (http://hg.savannah.gnu.org/hgweb/octave/rev/b22528fd3deb).  An important issue is that when Octave is in the midst of crashing it attempts to save all variables to the octave-workspace file.  If you happen to have a classdef object in the workspace then the saving of variables itself has an error and the entire workspace is lost.

Rik <rik5>
Group administrator
Thu 27 Aug 2015 06:51:00 PM UTC, comment #2: 

Also, would it make sense to change the error to just be a warning so that objects that could be saved are saved and the others are just skipped?

John W. Eaton <jwe>
Group administrator
Thu 27 Aug 2015 06:17:13 PM UTC, comment #1: 

In https://lists.gnu.org/archive/html/octave-maintainers/2014-08/msg00180.html, jwe mused whether adding support for classdef objects for all file formats might be a waste of time, and whether this would be a good time to deprecate some of the supported file formats, or only add support for new types to certain file formats, etc.

So load/save of classdef objects may or may not need to be implemented in all possible file formats, it may be worth further discussion.

Mike Miller <mtmiller>
Group Member
Thu 27 Aug 2015 12:09:04 PM UTC, original submission:  

Classdef objects cannot currently be loaded or saved:


>> x = inputParser;
>> save foo
error: octave_base_value::save_ascii(): wrong type argument 'object'
>> save -binary foo
error: octave_base_value::save_binary(): wrong type argument 'object'



Mike Miller <mtmiller>
Group Member

 

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

Attach Files:
   
   
Comment:
   

 

Carbon-Copy List
  • -email is unavailable- added by philipnienhuis (Posted a comment)
  • -email is unavailable- added by rik5 (Posted a comment)
  • -email is unavailable- added by cbm (Posted a comment)
  • -email is unavailable- added by dasergatskov (Posted a comment)
  • -email is unavailable- added by kolmanthomas (Updated the item)
  • -email is unavailable- added by mmuetzel (Updated the item)
  • -email is unavailable- added by seijikun
  • -email is unavailable- added by i7tiol (Posted a comment)
  • -email is unavailable- added by jwe (Posted a comment)
  •  

    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
    2025-07-11 mmuetzel Attached File- Added bug45833-part4-classdef-load-vector-v1.patch, #57395
    2025-07-11 mmuetzel Attached File- Added bug45833-part3-classdef-load-save-text-format-v4.patch, #57394
    2025-07-10 mmuetzel Attached File- Added bug45833-part3-classdef-load-save-text-format-v3.patch, #57391
    2025-07-09 mmuetzel Attached File- Added bug45833-part4-classdef-load-vector-v0.patch, #57389
    2025-07-09 mmuetzel Attached File- Added bug45833-part2-classdef-loadobj-saveobj-v2.patch, #57387
        Attached File- Added bug45833-part3-classdef-load-save-text-format-v2.patch, #57388
    2025-07-09 mmuetzel Attached File- Added bug45833-part2-classdef-loadobj-saveobj.patch, #57380
        Attached File- Added bug45833-part3-classdef-load-save-text-format.patch, #57381
    2025-07-07 mmuetzel StatusNone In Progress
    2025-07-07 mmuetzel Attached File- Added bug45833-classdef-load-save-v3.patch, #57373
    2025-07-05 rik5 Carbon-CopyRemoved 72865 -
    2025-07-04 kolmanthomas Attached File- Added load_save_patch2.diff, #57363
    2025-07-02 kolmanthomas Attached File- Added load_save_patch.diff, #57358
    2024-01-23 mmuetzel CategoryInterpreter Classdef
    2019-06-27 seijikun Carbon-Copy- Added seijikun
    2019-02-26 mtmiller Carbon-CopyRemoved 80942 -
    2018-08-06 mtmiller Dependencies- bugs #54455 is dependent
    2015-08-27 mtmiller Dependencies- bugs #45831 is dependent

    Back to the top

    Powered by Savane 3.15-e6e5.
    Corresponding source code