Interface refinements 2024-01

[apjanke]

Some misc interface refinements to consider.

• Mark table's rows, cols, and hasrownames as deprecated instead of removing immediately.
  • Maybe grpstats too?
    • Octave's missing-function hook may be a lighter weight way of doing this.
• Have iscategorical, isdatetime, etc. respect method overrides like istable does?
  • Check whether Matlab's interface works that way; maybe don't do it if it would be incompatible.
• Add NaS and NaC.
• Drop internal ismissing() use for unknown/generic types, add ismissing methods.
  • [no] Maybe make isnanny() private or +internal.
    • Can't do that – needs to be called polymorphically so classes can override it, so must be a free function.
• Move validator functions to a subdir, for tidier code browsing?
• Namespace [ ] proxyKeysForMatrixes, [X] tableOuterFillValue, and the like somehow, with a common name prefix, or a package or private-ization?
  • I think these are things I came up with, and not part of the standard M-code interface, so we're free to rename. Maybe they should be moved to a mixin interface as method overrides, and have the main functions become private/internal ones that respect those optional method overrides.
• Move +tblish/+chrono entirely under +tblish/+internal, since (currently) nothing in tblish.chrono is part of Tablicious's public interface? Same for +tblish/+table.
• Maybe make planar-class structural methods, and other methods that are hacks to work with core Octave's lack of support, Hidden, so they don't show up in the doco or tab completion?
• Remove pp(). (It's a convenience command I like personally; not a good fit for a library interface.)
  • Or at least move it to a separate "helper commands" dir that's not loaded by default.

Basically, get tighter about having the visible public (non-private, non-+internal) names in inst/ be just the stuff in Tablicious's public interface, or public-facing concrete classes.

See also

• Cleaning table classdef methods #115

[pr0m1th3as]

One major issue regarding maintenance is the sheer volume of methods in each classdef. I think we should make a list of what should remain and what not. A major obstacle for this is (as always) backwards compatibility. As we get to newer Octave versions, a lot of functions are available. For example, isstring is readily available since Octave 7.2, so there is no need for this to be in the string class as a method. There is also this huge list of functions under the % plot() and related functions. Yuck. and the % Planar structural stuff. I really don't see the necessity for these and it can become quite problematic once core devs start developing support for the classes in the Tablicious package.

[apjanke]

One major issue regarding maintenance is the sheer volume of methods in each classdef.

Oh yes. Most of these methods are just hacks so that these @string and @datetime classes can kind of pretend to be a built-in type and work with regular Octave functions, even in a version of Octave that does not have string or datetime support in core Octave. Once core Octave adds string support, most of these methods just go away.

For example, isstring is readily available since Octave 7.2, so there is no need for this to be in the string class as a method.

The isstring function is there, but it returns the wrong answer, for my purposes. It just returns false regardless of what its input is, I think, even if that input is a string array. Here's the whole isstring function impl from Octave 8.4:

function tf = isstring (s)
  if (nargin < 1)
    print_usage ();
  endif

  tf = false;
endfunction

The isstring method on Tablicious's string class is what makes the expression isstring(str) return true when str is a string array. Which is what you want, if you want the isstring function to be useful for programming with strings. That is its whole point: it returns true when the input is a string and false when the input is not a string. But the current core Octave version is only handling half of that job.

I think we should make a list of what should remain and what not.

Yeah, that's a good idea. Things that are part of the "real" string interface and would continue to be supplied by that class, and what are compatibility hacks.

% Planar structural stuff.

That "planar structural" stuff is all necessary in M-code classdef classes if you want to implement an efficient "planar-organized" class, instead of a slower, more-memory-using "struct-organized" class. (AFAIK.) If string were a built-in type instead of a classdef class (and IMHO it sure should be), then that all goes away, or rather, is supplied in some manner by the built-in type, in hopefully a less-ugly manner. Not a huge deal for string since the underlying field is a cell anyway, but makes a big difference for classes like categorical and datetime with numeric underlying fields. Basically, struct-organized classes defeat (true) vectorization.

For string and categorical in particular, and probably/hopefully datetime, I would recommend that the core Octave developers just ignore everything in Tablicious's class definitions for them, and instead create some true built-in type support for them, like there is for double and char and cell. Would be nicer code, and probably better performance & memory efficiency, which is nice for such fundamental types as these. I only wrote them as classdefs because I'm not very good at C++, I don't know if user-defined Octave packages can even supply new built-in types with C++ like that, and I kinda just wanted to get a prototype of table plus these support classes out there to prove they could be useful, not that I think that's a good way to do them overall.

[pr0m1th3as]

• Drop internal ismissing() use for unknown/generic types, add ismissing methods.

I think the ismissing() method should stay at least in the string class

[apjanke]

Drop internal ismissing() use for unknown/generic types, add ismissing methods.

I think the ismissing() method should stay at least in the string class

Oh, it will. For this item, I mean that inside the implementation code of Tablicious's functions and methods, it will not try calling ismissing(x) on an x of unknown type, or of a type that is not one that Tablicious supplies and defines an ismissing method for. Here, I mean drop the dependency on or use of ismissing() for arguments/values that it may not exist for, so we don't have a hard dependency on an ismissing function as opposed to ismissing methods on specific classes, and can safely remove the top-level ismissing function from Tablicious, yet keep its dependency on Statistics optional and limited to stats-related functionality.

[pr0m1th3as]

We can keep it for now, and we could remove it at a later point once the statistics ismissing function is able to handle string and other classes as well. The truth is that ismissing, fillmissing, standardizeMissing and rmmissing functions belong to core Octave, despite they were moved to the statistics package before I took over its maintenance. I would assume (and suggest) that at the time that table, string and other related classes are mature enough to merge into core Octave, these 4 functions will also transition to core Octave.

[apjanke]

I would assume (and suggest) that at the time that table, string and other related classes are mature enough to merge into core Octave, these 4 functions will also transition to core Octave.

That sounds right to me.

[pr0m1th3as]

Can you merge bulbasaur-24 branch to main so I can make a new fork from latest dev and branch locally to do modifications to table classdef? Working on a remote brach of a repo without owner permissions is quite complex and makes my life quite miserable. This way, I will be able to make more focused commits instead of a single large one as you said earlier (in this thread or another, I can't recall) and then make a single PR to your main brach without missing the local commit history.

[apjanke]

Can you merge bulbasaur-24 branch to main

Already done; main is caught up with bulbasaur-24 as of late last night US time. And I'll stop doing rebases of bulbasaur-24 so your tracking copy of it will be stable; I'll keep rebases on separate branches.

[pr0m1th3as]

Ok thanks, I 'll make a clean fork and start working from there.

[pr0m1th3as]

I 've noticed that in some parts of the code you close statements with end and in other parts you use the more Octave like if...endif, for...endfor. Do you mind if we stick with the second code style and eventually adjust the all the code in Tablicious to this Octave like format?

[apjanke]

Do you mind if we stick with the [endif, endfor] code style [...]

Not at all. I kinda intended to adopt that Octave-standard endif/endfor/endXXX code style for this project, but I'm really used to the Matlab plain-end style so have had some trouble being consistent about it. Any plain-ends in the code are just oversights on my part, and can & should be changed to the more-specific versions. Feel free to do so in PRs, and I'll change them as I notice them myself.

(In some other Octave projects, I've stuck with end for Matlab compatibility, but there's no reason for that here.)

If I find some spare time tonight, I'll go through and change them all in one monster commit to get it over with and keep the commit history tidy.

[apjanke]

An speaking of code style, what editor(s) are you using, @pr0m1th3as? I'm wondered what introduced all those remove-trailing-whitespace changes in your recent PR. I'm wondering if I could set up an .editorconfig or similar to make consistent editor configuration easier.

And I actually prefer the no-trailing-whitespace formatting like you did there, so I'm thinking maybe I'll convert all the files in the repo to that, to establish a baseline. But I'm not sure how the Octave desktop editor behaves and whether it would work well with that.

[pr0m1th3as]

I am always using the Octave editor for Octave code. It is by default that it removes all trailing white spaces every time you save a file. The other code style is that tab is always 2 white spaces and not a tab per se. These are the default settings for all Octave core code as well.
Don't push any changes in the table classdef file yet because it will brake all changes I am currently working on. It won't have any trailing white spaces any way, because I am editing with the Octave editor.

[pr0m1th3as]

Other changes related to Octave code style is that ## is used for comments instead of %. The latter is used for identifying demo and test block at the end of the file. Two major code style changes in your code is

1. using ! instead of ~ for negation.
2. using () for if, while, switch statements.
   for example

if (x == y || x == 1)
...
endif

[pr0m1th3as]

If you check the statistics code style is fully compatible with Octave core M code with the sole exception that I use . at the end of the error messages. IMHO, the more Octave code stye compatible Tablicious is, the better, since it will be much easier to core dev to merge the classdefs into core when the time comes.

[apjanke]

I agree. I'll convert Tablicious over to Octave's code style. Here's a separate ticket for it: #116.

I'll wait until you're done with that table work before doing changes to that file.

[apjanke]

Here's a "WIP/swift-style" branch with a preview of what the code style conversion to GNU Octave style would look like: https://github.com/apjanke/octave-tablicious/tree/WIP/swift-style. It does a mass restyling addressing most of the items noted in #116. I didn't do adding (...) around the if and while conditions yet, because that's harder to automate with a regex.

Don't worry about reconciling your table work with these changes: they're on a separate branch, and I'll just rebase and adapt them to your changes once they're in.