patchGNU Octave - Patches: patch #10102, [octave forge] (statistics) Add...

 
 

patch #10102: [octave forge] (statistics) Add new 'missing' functions: ismissing, rmmissing, standardizeMissing, fillmissing

Submitted by:  None
Submitted on:  Mon 30 Aug 2021 09:30:45 AM UTC
 
Category:  Forge : new function Priority:  5 - Normal
Status:  Ready For Test Privacy:  Public
Assigned to:  None Originator Email:  -email is unavailable-
Open/Closed:  Open
* Mandatory Fields

Add a New Comment (Rich Markup)
   

Comment Type & Canned Response:
       

 

( Jump to the original submission )

Mon 16 May 2022 05:45:22 AM UTC, comment #24: 

attached are updated ismissing and rmmissing functions that are compatible with Matlab 2022a, and also correct a corner case in ismissing where NaN is supplied as an indicator. Also attached fillmissing is complete, including all of the many corner cases I could find. Turs out the optional second output has many inconsistencies between methads that I wouldn't be surprised to see Matlab change in upcoming versions. If it matters, a future effort can pare down the BIST count to identify any redundant tests. The numerous method codepaths made it easier to duplicate tests for each path.

these updated files are pushed as:
https://sourceforge.net/p/octave/statistics/ci/b7d3493949a7

updated status to Ready for Test. also renamed to remove 'missing' from the patch description.  probably best to move 'missing' implementation to a separate feature request report if anything, as it may be a while before classdef is able to support a dynamic typing variable as described in comment #13.

(file #53210, file #53211, file #53212)

Nicholas Jankowski <nrjank>
Project Member
Mon 02 May 2022 08:55:31 PM UTC, comment #23: 

uploading a 99% feature complete fillmissing to capture progress.  it's big. there's probably a lot of code recduction that could be done if I planned out function blocks better in advance, but it works. And I apologize for the BIST count. I added them as I went along for each specific input parameter variation and kept finding corner cases.

then I noticed after an upgrade to matlab 2022a while vetting tests for compatability that behavior had changed. see https://www.mathworks.com/help/matlab/release-notes.html

ismissing, rmmissing, and groupsummary Functions: Accept data types with no standard missing value
The ismissing syntax ismissing(A) now returns logical 0 (false) when the input data type has no default definition of a standard missing value.

rmmissing and the nummissing and nnz methods of groupsummary no longer error for input data types with no default definition of a standard missing value.

An example of code that used to error but now executes is:

A = [struct struct struct];
TF = ismissing(A)
TF =
    1x3 logical array
    0    0    0

so with that I'll go back and review input behavior for fillmissing, ismissing and rmmissing to see what's still compatible. then i'll probably work up a 'good enough to close this report' patch.

(file #53168)

Nicholas Jankowski <nrjank>
Project Member
Fri 18 Mar 2022 09:17:05 PM UTC, comment #22: 

just uploading a partial implementation of fillmissing attached. let it sit for a while, but as suspected all of the different method options, including different handling of middle-versus-end missing values made this a bit more convoluted. Also, thought I could just mass most of the methods off to interp1 (since the methods seem drawn from interp1), only to find out interp1 fails when there's only 1 data point in that column for some methods while fillmissing doesn't. Oh, octave also still doesn't have a makima implementation. when it does this will work, but currently i just have interp1 issuing the error so nothing has to change when it gets added. that could probably be cleaned up.

anyway, the helptext is currently complete for eventhing that should get finished, and that don't rely on currently unimplemented classes. all of the property-value pairs are implemented, (samplepoints, endvalues, missinglocations, maxgap), all of the standard 'methods' are implemented. if i changed the error handling, the way I did it could expand the 'endvalues' handling to include all options independently of whats' done for the rest of the array, extending the function a bit beyond what matlab's does. may look at that at the end.

what's still missing is just the moving window methods and the fill-function method.

Once those are done i'll consider this complete (but far from optimized) for Octave's current class-set.  lots of code could probably move into subfunctions, but lately we've notice a lot of function call overhead I was trying to avoid, in addition to being lazy about looking for repeat code. lots of BISTs for all of the different option combinations. I kept finding exceptions so kept adding. some of the redundant ones could be trimmed later.

m-file attached for anyone needing this function in partial form before i get around to finishing it.

(file #53002)

Nicholas Jankowski <nrjank>
Project Member
Wed 22 Dec 2021 09:40:04 PM UTC, comment #21: 

updated the news file for 1.4.4, and pushed the ismissing/rmmissing/standardizeMissing changes to:

https://sourceforge.net/p/octave/statistics/ci/427b93d66b774b7da542ef699738096e04d362ff/

Nicholas Jankowski <nrjank>
Project Member
Wed 22 Dec 2021 07:45:14 PM UTC, comment #20: 

Ok will do. Saw 1.4.3 was out, but news wasn’t bumped to 1.4.4 and didn’t see a bookmark like for 1.4.2. Didn’t know if there was any other maintenance items to be done.  I’ll push those and update news later today.

Nicholas Jankowski <nrjank>
Project Member
Wed 22 Dec 2021 05:11:49 PM UTC, comment #19: 

Feel free to push your changes and update the NEWS file. 1.4.3 has already been released.

Nir Krakauer <nir_krakauer>
Fri 17 Dec 2021 10:04:05 PM UTC, comment #18: 

ok. noticed a typo in ismissing and the other in standardizeMissing. this patch includes the corrected ismissing, updates to rmmisssing, and adds the corrected standardizeMissing. (No changes made to the use of ismember yet, so the code is simple but it's not fast.).

fillmissing is pending but it'll take a bit longer with all of the options. will create a separate patch for that later that will depend on this one (for ismissing). at that point i'll also attached a zip with the four functions files in their fully working form.

(file #52522)

Nicholas Jankowski <nrjank>
Project Member
Fri 17 Dec 2021 08:20:27 PM UTC, comment #17: 

ok, attached is a patch to update ismissing. lots of matlab verified BISTs added, passes them all.

again, can push once it's clear pkg has been processed for next release, but e.g., still showing 'for 1.4.3 in news). adding the updated m-file for availability.

(file #52520, file #52521)

Nicholas Jankowski <nrjank>
Project Member
Fri 17 Dec 2021 05:56:28 PM UTC, comment #16: 

also, doing some time testing the loops in ismissing (some have faster alternatives, some are suprisingly fast), I'm noticing just how terribly slow the 'ismember' line is in standardizeMissing. It's nice and simple and clean, but might look for alternatives there

Nicholas Jankowski <nrjank>
Project Member
Fri 17 Dec 2021 02:36:50 PM UTC, comment #15: 

good catch, i'll fix it.  should just say empty string.

I also was working on fillmissing, planning on having it call ismissing.  it's a fairly hefty function with all of the options, but I then noticed that there are some issues with ismissing.  Added a bunch of other tests for function and matlab compatibility and am working up a patch for that as well. Should get that up this weekend. will check rmmissing too. fillmissing might take a little longer depending on how many of the options I try to include.

Nicholas Jankowski <nrjank>
Project Member
Fri 17 Dec 2021 02:14:35 PM UTC, comment #14: 

comment #13:

> I've attached a patch for standardizeMissing here (doesn't update NEWS as that doesn't seem to be updated since the 1.4.3 release yet). 
>


I think there is a mistake in the texinfo. It reads:

{''} (white space in cell): string cells.

while it actually is an empty string, without spaces. The code is correct, the problem is just with the texinfo.

Anonymous
Thu 16 Dec 2021 04:55:19 PM UTC, comment #13: 

I think this is worth keeping open to track implementation of the different functions related to 'missing' rather than creating other reports.

note that 'missing' is a class that automatically supplies the appropriate missing value (NaN, '', etc) based on the class of the object being substituted into.  it can't be implemented as a classdef until converter methods are fixed (see bug #61676). It may be able to be created as an old @ style class. in any case, it would greatly simplify the code in many of the other functions, but doesn't seem to be a block to doing so.

I've attached a patch for standardizeMissing here (doesn't update NEWS as that doesn't seem to be updated since the 1.4.3 release yet). 

I might take a stab at fillmissing soon as well.

(file #52516)

Nicholas Jankowski <nrjank>
Project Member
Fri 03 Dec 2021 08:23:26 PM UTC, comment #12: 

Shall we leave this open as "postponed" with low priority until ismissing/rmmissing also appear in core Octave?

Philip Nienhuis <philipnienhuis>
Project Member
Thu 02 Dec 2021 03:05:48 PM UTC, comment #11: 

ok great.  I'm honestly not familiar with package folder layout.  I don't see anything about 'conditional' installation and the different folders at https://wiki.octave.org/Category:Packages or https://octave.org/doc/latest/Creating-Packages.html 

is it covered anywhere?

Nicholas Jankowski <nrjank>
Project Member
Thu 02 Dec 2021 02:54:36 PM UTC, comment #10: 

Thanks, I changed the subdirectory from install-conditionally/distributions to /base and pushed it
https://sourceforge.net/p/octave/statistics/ci/7950d2715a5c55e822b2365b32e5fa90bd502d3a/

Nir Krakauer <nir_krakauer>
Thu 02 Dec 2021 02:20:28 AM UTC, comment #9: 

patch for those two functions into install-conditionally/distributions with NEWS update.

I can now push things to OF, but didn't with this yet because I haven't actually checked the functions yet, just grabbed them from the other patches made for core.

(file #52403)

Nicholas Jankowski <nrjank>
Project Member
Thu 02 Dec 2021 01:45:18 AM UTC, comment #8: 

We can add these functions to the Statistics package if someone prepares a patch. Should they be in the install-conditionally subdirectory to allow for being eventually added to core Octave?

Nir Krakauer <nir_krakauer>
Thu 25 Nov 2021 12:44:23 AM UTC, comment #7: 

Good point. Also, at this point making it into v7.1 is slim, so having them in statistics would make them more readily available between now and v8

Nicholas Jankowski <nrjank>
Project Member
Wed 24 Nov 2021 08:39:39 PM UTC, comment #6: 

@Nicholas:
Even if they're accepted in core Octave (as I think they should be) they also need to be in the statistics package for some time to cater for Octave <= 6.4.0 if functions like e.g., pca.m in the statistics package are to be dependent on them.

Philip Nienhuis <philipnienhuis>
Project Member
Wed 24 Nov 2021 04:27:42 PM UTC, comment #5: 

checking my copy of matlab, it lists ismissing as under "\toolbox\matlab\datafun".  but after testing it, license('inuse') still just shows MATLAB.  searching in the Add-On Explorer for 'ismissing', it shows it as present in MATLAB, and shadowed in the Statistics and ML Toolbox, and Text Analytics Toolbox. (maybe this wasn't always the case.)  rmmissing ONLY shows up under core MATLAB.

I also note that there are missing, standardizeMissing, and fillmissing.

missing: MATLAB, Signal Proc, Financial toolbox
standardizeMissing: only MATLAB
fillmissing: MATLAB, financial toolbox

Based on that, I agree ismissing and rmmissing seem like general data processing functions and should be in core octave, and the other three as well.  (If i'm not misunderstanding, I think the /datafun toolbox is just a standard matlab 'bundled' toolbox considered part of core.)

Nicholas Jankowski <nrjank>
Project Member
Sun 05 Sep 2021 05:48:49 PM UTC, comment #4: 

Thanks.
It'll take a little time, unforeseen busy times at work.

After a quick glance I think the patches look good. However, in the scripts directory in help/__unimplemented__.m the stanzas about rmmissing and ismissing should be dropped as well.

Before you adapt that, let's first get the opinion of a core developer, in particular the one who some years ago move loads of the Octave core statistics functions to the statistics package (with good reason, many of those functions needed and still need overhaul and are too much of a maintenance burden for Octave).
IMO ismissing and rmmissing should be in core - they are quite useful outside the field of statistics as well.

>> ismissing
error: 'ismissing' undefined near line 1, column 1

The 'ismissing' function belongs to the statistics package from Octave
Forge which you have installed but not loaded.  To load the package, run
'pkg load statistics' from the Octave prompt.

Please read <https://www.octave.org/missing.html> to learn how you can
contribute missing functionality.

Philip Nienhuis <philipnienhuis>
Project Member
Sun 05 Sep 2021 10:39:09 AM UTC, comment #3: 

I made two separate patches for rmmissing and ismissing. The patches should be fully compliant.

(file #51870, file #51871)

Anonymous
Wed 01 Sep 2021 04:21:45 PM UTC, comment #2: 

OK, I made a patch.

comment #1:

> I wonder why rmmissing.m contains a switch statement where an if/elseif block would suffice; is it to be able to extend to string classes, if ever implemented in core Octave?
>


That was the diea.

comment #1:

> Additionally I think there's no harm in extending beyond Matlab compatibility; I can see use cases for mixed-type cell arrays (e.g., output of csv2cell() or xlsread.m and the like).
>


Interesting, maybe later. For now I'd like to see 'pca' and its requirements implemented.

(file #51860)

Anonymous
Tue 31 Aug 2021 07:35:10 PM UTC, comment #1: 

Thanks ilguido.

These are core Matlab functions (since r2013a) and so I think they should go into core.

I wonder why rmmissing.m contains a switch statement where an if/elseif block would suffice; is it to be able to extend to string classes, if ever implemented in core Octave?

Additionally I think there's no harm in extending beyond Matlab compatibility; I can see use cases for mixed-type cell arrays (e.g., output of csv2cell() or xlsread.m and the like).

For core Octave some more rigor is required. Adding BIST for real operations rather than just error checks is imperative. There are also some slight style things (core devs are a bit strict), e.g. single quotes around a text in a case condition in rmmissing; empty line before endfunction; and I'm unsure of the texinfo.
The copyright owner would be "The Octave Project Developers" but you'd me mentioned as author in the Mercurial log so eternal fame would be preserved.
I hope you'll agree with this.

For core Octave (and actually the statistics package as well) we use Mercurial rather than git. But if you would first improve ismissing and rmmissing I'll help in producing a mercurial cset.

Philip Nienhuis <philipnienhuis>
Project Member
Mon 30 Aug 2021 09:30:45 AM UTC, original submission:  

These are the functions ismissing and rmmissing, for finding and removing missing data from a set (vector, matrix etc.).

I am not sure about the package they belong to.

Example:

octave:1> a = [1 2 3 NaN 4 5 NaN 7 8];

octave:2> ismissing(a)
ans =

  0  0  0  1  0  0  1  0  0

octave:3> rmmissing(a)
ans =

   1   2   3   4   5   7   8

Anonymous

 

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

Attach Files:
   
   
Comment:
   

Attached Files
file #53210:  fillmissing.m added by nrjank (109KiB - text/plain - "final" compatible versions)
file #53211:  rmmissing.m added by nrjank (7KiB - text/plain - "final" compatible versions)
file #53212:  ismissing.m added by nrjank (7KiB - text/plain - "final" compatible versions)
file #53168:  fillmissing.m added by nrjank (94KiB - text/plain - mostly feature complete fillmissing.m)
file #53002:  fillmissing.m added by nrjank (64KiB - text/plain - partial implementation)
file #52522:  missingfunctions_update.diff added by nrjank (22KiB - application/octet-stream - corrected ismissing, rmmissing, and adds standardizeMissing, all in one patch.)
file #52520:  immissing_update.diff added by nrjank (8KiB - application/octet-stream - updated ismissing function and patch with corrected array and compatible input handling.)
file #52521:  ismissing.m added by nrjank (6KiB - text/plain - updated ismissing function and patch with corrected array and compatible input handling.)
file #52516:  statistics_standardizeMissing.diff added by nrjank (8KiB - application/octet-stream)
file #51870:  ismissing.diff added by None (5KiB - text/x-patch - new files, NEWS)
file #51871:  rmmissing.diff added by None (7KiB - text/x-patch - new files, NEWS)
file #51860:  ismissing_rmmissing.diff added by None (11KiB - text/x-patch - hg export -r tip)
file #51836:  ismissing.m added by None (4KiB - text/x-objcsrc)
file #51837:  rmmissing.m added by None (5KiB - text/x-objcsrc)

 

Digest:
   bug dependencies.

Items that depend on this one: None found

 

Carbon-Copy List
  • -email is unavailable- added by nir_krakauer (Posted a comment)
  • -email is unavailable- added by nrjank (Updated the item)
  • -email is unavailable- added by philipnienhuis
  • -email is unavailable- added by philipnienhuis
  • -email is unavailable- added by None (Submitted the item)
  •  

    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 logged-in users can vote.

     

     

     

     

    Follow 23 latest changes.

    Date Changed by Updated Field Previous Value => Replaced by
    2022-05-16 nrjank Attached File- => Added fillmissing.m, #53210
        Attached File- => Added rmmissing.m, #53211
        Attached File- => Added ismissing.m, #53212
        StatusIn Progress => Ready For Test
        Summary[octave forge] (statistics) Add new 'missing' functions: missing, ismissing, rmmissing, standardizeMissing, fillmissing => [octave forge] (statistics) Add new 'missing' functions: ismissing, rmmissing, standardizeMissing, fillmissing
    2022-05-02 nrjank Attached File- => Added fillmissing.m, #53168
    2022-03-18 nrjank Attached File- => Added fillmissing.m, #53002
    2021-12-17 nrjank Attached File- => Added missingfunctions_update.diff, #52522
    2021-12-17 nrjank Attached File- => Added immissing_update.diff, #52520
        Attached File- => Added ismissing.m, #52521
    2021-12-16 nrjank Summary[octave forge] (statistics) Add new functions ismissing and rmmissing => [octave forge] (statistics) Add new 'missing' functions: missing, ismissing, rmmissing, standardizeMissing, fillmissing
    2021-12-16 nrjank Attached File- => Added statistics_standardizeMissing.diff, #52516
        Dependencies- => Depends on bugs #61676
    2021-12-02 nrjank Attached File- => Added statistics-add-immissing-rmmissing-patch10102.diff, #52403
        StatusNeed Info => In Progress
    2021-11-23 nrjank StatusNone => Need Info
    2021-09-05 philipnienhuis Carbon-Copy- => Added rik5
        Carbon-Copy- => Added philipnienhuis
    2021-09-05 None Attached File- => Added ismissing.diff, #51870
        Attached File- => Added rmmissing.diff, #51871
    2021-09-01 None Attached File- => Added ismissing_rmmissing.diff, #51860
    2021-08-30 None Attached File- => Added ismissing.m, #51836
        Attached File- => Added rmmissing.m, #51837

    Back to the top


    Powered by Savane 3.9