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

 
 

patch #10102: [octave forge] (statistics) Add new 'missing' functions: missing, 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:  In Progress Privacy:  Public
Assigned to:  None Originator Email:  -email is unavailable-
Open/Closed:  Open

Add a New Comment (Rich Markup)
   

You are not logged in

Please log in, so followups can be emailed to you.

 

( Jump to the original submission)

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 #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)
  •  

    Do you think this task is very important?
    If so, you can add your encouragement to it.
    This task has 0 encouragements so far.

    Only logged-in users can vote.

     

     

     

    Follow 16 latest changes.

    Date Changed by Updated Field Previous Value => Replaced by
    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.7