Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

octave/matlab differences for methods(<class>) #87

Open
cbm755 opened this issue Jun 15, 2015 · 10 comments
Open

octave/matlab differences for methods(<class>) #87

cbm755 opened this issue Jun 15, 2015 · 10 comments

Comments

@cbm755
Copy link
Collaborator

cbm755 commented Jun 15, 2015

In Symbolic, I have a @logical dir which adds a single method (isAlways) to the logical class.

  • In Matlab, methods('logical') gives all sorts of stuff abs, all, amd, any, etc and including my isAlways method.
  • In Octave, methods('logical') gives just isAlways.

Maybe this is an upstream Octave bug.

But even in the Matlab case, should doctest then call doctest on all of these methods? Even those not defined in this 'project'? This is good for a user, but for package maintainer, I want to doctest my local stuff...

@catch22
Copy link
Collaborator

catch22 commented Jun 15, 2015

That's a good question. I believe the intuitive thing might be to just test those methods that are explicitly documented in the class definition (after all, this is doctest 😄).

At some point we might then want to add an explicit directive that tells doctest to test the overriding method with the overridden method's doctests (but let's do this when the problem arises).

@oheim
Copy link
Collaborator

oheim commented Jun 15, 2015

https://savannah.gnu.org/bugs/?func=detailitem&item_id=44390

However, it makes sense to only test methods that actually are overridden in the subclass and contain new test cases.

@catch22 catch22 added this to the 0.4.1 milestone Sep 22, 2015
@cbm755
Copy link
Collaborator Author

cbm755 commented Oct 31, 2015

I don't think we need this for a point release. I'll drop the milestone

@cbm755 cbm755 removed this from the 0.4.1 milestone Oct 31, 2015
@cbm755
Copy link
Collaborator Author

cbm755 commented Nov 25, 2017

There are two slightly different things being discussed here:

  1. class C is a subclass of P. The discussion is whether doctest C should test methods defined only in P.
  2. a "local class directory" that "monkey-patches" new methods onto an existing class. This was my original doctest logical example. There I feel doctest logical might do something different depending on whether there is a local @logical directory or not. Or maybe doctest logical might have different behaviour than doctest @logical.

@catch22
Copy link
Collaborator

catch22 commented Nov 27, 2017

Thanks for clarifying.

Intuitively, I would expect that only doctests defined explicitly in files the local @P / @logical directories are tested.

@apjanke
Copy link
Contributor

apjanke commented Mar 17, 2019

IMHO this methods() behavior is definitely an upstream Octave bug. If it's supposed to be Matlab M-code compatible, methods() should return all the methods that are dispatchable on an object or type, regardless of whether they were inherited. Should probably code doctest with the expectation that this behavior may change in the future, and if you want compatibility across Octave versions you'll need a compatibility shim or a different source of the methods list. Maybe .MethodsList from meta.class?

@cbm755
Copy link
Collaborator Author

cbm755 commented Mar 17, 2019

How is that supposed to work? Is it only for classdef?

>> m = meta.class.fromName(sym)
error: wrong type argument 'class'
error: fromName: CLASS_NAME must be a string
>> m = meta.class.fromName('sym')
m = [](0x0)

@apjanke
Copy link
Contributor

apjanke commented Mar 18, 2019

Aw, darn. Yeah, I guess it's only for classdef or traditional user-defined classes, not built-in types. I don't know if this is a deviation from Matlab behavior.

@cbm755
Copy link
Collaborator Author

cbm755 commented Mar 18, 2019

sym is a user-defined class... Am I doing it right?

@apjanke
Copy link
Contributor

apjanke commented Mar 20, 2019

You're doing it right. Looks like Octave’s meta.class only supports new-style classdef (“MCOS”) classes, not old-style “@ directory” classes. I’d call that a bug in Octave. But it does mean that meta.class won’t be usable here, at least for the time being.

Maybe doctest is stuck searching the path and doing its own class definition parsing if it wants to be thorough, and also not have the methods() behavior of including inherited methods.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants