symbolic icon indicating copy to clipboard operation
symbolic copied to clipboard

[wip] Implement @sym/class to return python class name of @sym vars

Open genuinelucifer opened this issue 8 years ago • 16 comments

fixes issue #549

Currently it does not follow octave's builtin class symantics. I have kept a full flag just for discussion.

genuinelucifer avatar Sep 20 '16 07:09 genuinelucifer

it breaks most of the doctests currently. I'll update the doctests once this is finalised.

genuinelucifer avatar Sep 20 '16 07:09 genuinelucifer

Hi, this is very nice! it seems works fine with syms, i think left symfuns, you will need improve this to can set class (values and all that things):

>> syms g(t)
error: Invalid call to class.  Correct usage is:

 -- Method on C: = class (X)
 -- Method on C: = class (X, FULL)
error: called from
    print_usage at line 90 column 5
    class at line 51 column 5
    symfun at line 184 column 5
    syms at line 176 column 10

from symfun.m line 184:

  f = class(f, 'symfun', expr);

sadly we can't use the builtin function in symfun:

  f = builtin('class', f, 'symfun', expr);

octave don't likes this, don't recognizes the call as the type (@symfun/symfun):

>> syms g(t)
error: class: 'symfun' is invalid as a class name in this context
error: called from
    symfun at line 184 column 5
    syms at line 176 column 10

its very probable this can be a octave bug...

And maybe instead of this line:

(nargin == 2 && !islogical(full))

remove it and change the python cmd line to:

cname = python_cmd (cmd, x, logical(full));

i think its very normal use 0 and 1 as logical values, only to enable that.

Thx. Cya.

latot avatar Sep 20 '16 15:09 latot

i think left symfuns, you will need improve this to can set class (values and all that things): i think its very normal use 0 and 1 as logical values, only to enable that.

Hi @latot . Sure I'll incorporate these changes.

genuinelucifer avatar Sep 20 '16 18:09 genuinelucifer

I just wanted to note that I tried a few different things but as I am not able to find a decent way to make it work for both syms and symfuns... symfunc calls class at one point in the constructor and if I make a @sym/class.m then that is called. Which causes the creation of symfunc to fail. Also, in python I am getting a dict when I pass the symfunc! I probably have lost track of how symfuncs are being created... Moreover if I return any string from class say using:

if (isa (x, 'symfunc'))
   cname = 'symfunc';
end

then if I try syms g(t) then g is actually the string symfunc!! Also, as @latot mentioned, calling builtin('class',... also seems to be not working! . This probably would be a lot easier to fix on pytave IPC. If I don't get a way around this in windows, I will update the file to only work with pytave IPC and directly return the pyobject instead (which I hope should be fairly straightforward)...

genuinelucifer avatar Sep 23 '16 20:09 genuinelucifer

  1. Does implementing @symfun/class help? (I guess not)
  2. The ctor call to class may not be with classdef... Maybe we can do this class override after we switch to classdef?
  3. We should take the f = builtin('class', mystruct, 'symfun', myparentobject) thing to upstream Octave. Can you explain it or do you want me to file it?

cbm755 avatar Sep 27 '16 17:09 cbm755

I already report it, if you can complement the explanation it would be great:

https://savannah.gnu.org/bugs/?49169

Thx. Cya.

latot avatar Sep 27 '16 17:09 latot

The idea its can overload class only to return a value, don't to set (when we will set we call builtin).

latot avatar Sep 27 '16 17:09 latot

https://savannah.gnu.org/bugs/?49169

Commented upstream, hopefully we can get some information on what Matlab does in this situation.

Does implementing @symfun/class help?

No, the error is in overload resolution and the extra arguments passed when doing inheritance with old-style class-based classes.

The ctor call to class may not be with classdef

Right, classdef would work around this.

mtmiller avatar Sep 27 '16 18:09 mtmiller

mm, personally i think this @sym/class helps a lot, i normally works with a mix of things, syms, symfuns, intervals and matrix, and when i display the info its too hard know what are matrix and what are intervals(when i have a lot of data)....

latot avatar Sep 29 '16 22:09 latot

Right, classdef would work around this.

I will then try to do the classdef implementation now. If that works then we could implement this on top of that.

genuinelucifer avatar Sep 30 '16 14:09 genuinelucifer

@genuinelucifer and @cbm755 a little question, i don't have very clear what are differences between the actual sym and implement classdef (i try search but i can get a clear answer), ex. why class should works from classdef and not now? or what the differences between the concepts vs functionalities?

The mains differences i have clear are, you can set methods from classdef (but we can do this too from syms in other files), in classdef you can set properties to the objects (in some way sym do this with a struct), the extras of classdef are the events and enumeration but this last two don't are available in octave (and i was start thinking how use it :D).

This can approached from other way, edit the display function and add the actual class code there.

Thx. Cya.

latot avatar Sep 30 '16 14:09 latot

I'm not clear on differences either. I think of them as two different approaches to OO, or at least two different implementations. And so there are some differences between them, different bugs too ;-)

Classdef-style classes allow more OO features like static methods.

cbm755 avatar Sep 30 '16 16:09 cbm755

@latot where you working on the classdef? @genuinelucifer if you start too, let's get a "WIP" PR up ASAP for collaboration and not duplicating effort.

cbm755 avatar Sep 30 '16 16:09 cbm755

I'm not working on it now, for i can help i need first understand the concept.

Thx. Cya.

latot avatar Sep 30 '16 16:09 latot

Thanks @latot: your review if @genuinelucifer does it would be very much appreciated (and good chance to learn).

cbm755 avatar Sep 30 '16 16:09 cbm755

if you start too, let's get a "WIP" PR up ASAP for collaboration and not duplicating effort.

Definitely. I am compiling octave on my linux machine. I can probably send a WIP PR by tomorrow. I also have to go through classdefs once.

genuinelucifer avatar Sep 30 '16 17:09 genuinelucifer