bugGNU Octave - Bugs: bug #46856, Add 'stable' flag for `unique` and...

 
 

bug #46856: Add 'stable' flag for `unique` and related fiunctions?

Submitter:  Luis Mendo <lmendo>
Submitted:  Fri 08 Jan 2016 11:56:17 PM UTC
   
 
Category:  Octave Function Severity:  1 - Wish
Priority:  3 - Low Item Group:  Feature Request
Status:  Fixed Assigned to:  None
Originator Name:  Luis Mendo Open/Closed:  * Closed
Release:  * dev Operating System:  * Any
Fixed Release:  None Planned Release:  None
* Mandatory Fields

Add a New Comment Rich Markup
   

Jump to the original submission

Tue 06 Aug 2019 05:03:28 AM UTC, comment #13: 

I went and implemented the 'stable' option for all of the set functions.  The starting changeset is 27291:c4f9a0f097a3.  This will be a part of the 6.1 release of Octave.

Marking as fixed and closing report.

Rik <rik5>
Group administrator
Sat 26 Nov 2016 12:08:28 AM UTC, comment #12: 

See also bug #38460

kire pudsje <kpc>
Fri 11 Mar 2016 09:00:40 PM UTC, comment #11: 

I've modified  the set functions so that now they produce row ouput if first two inputs are rows

Luis Mendo <lmendo>
Thu 11 Feb 2016 01:20:10 PM UTC, comment #10: 

Thanks, Luis.  These look like great progress.

Some quick comments about formatting:
- Octave's style is to use "endif", "endfunction" etc. instead of just "end".
- Octave's style is not to have multiple statements on one line.  Every ";", "if", "end..." etc should be on its own line.
- Indentation is by two spaces, not four.
- The bracket of a function call has a space before it.
- "!" is used for not instead of "~".
- The entire body of the function is indented.
- The conditions of "if", "while" etc. have brackets around them.

Many of the guidelines are state at


https://www.gnu.org/software/octave/doc/interpreter/Octave-Sources-_0028m_002dfiles_0029.html#Octave-Sources-_0028m_002dfiles_0029


When I first started submitting patches, I found the emphasis on formatting frustrating, but I can now see how much easier it is to read code when it is uniform.

I don't think you need to use varargin.  You can just specify the names of the variables (A, B, str1, str2), and if it is called with fewer variables, then the later ones just have an undefined value.  That will make the code easier to read.

The code seems to provide all of the outputs provided "stable" isn't specified, so no functionality is lost.  That is good.

Leaving the C++ to do the hard work is also sensible; we can just rename the built-in so that we don't have these wrappers sharing the same names.

Matlab has introduced 'legacy', in addition to 'stable'.  The main difference seems to be in the second output, so it doesn't affect us yet, but we may want to handle that before the patches get applied.

Thanks for your contribution.

Lachlan Andrew <lachlan>
Wed 10 Feb 2016 08:50:49 PM UTC, comment #9: 

Sorry for the delay. I have attached my modified functions for `union`,  `intersect`, `setdiff`,  `setxor`. They add support for the `'stable'` input flag, but with the following limitations:

- They only work in the single-output case
- They are not a replacement for the builtin functions. Rather, they intercept them, do the necessary changes, and call the builtin functions to do most of the work

(file #36302, file #36303, file #36304, file #36305)

Luis Mendo <lmendo>
Thu 21 Jan 2016 11:04:07 PM UTC, comment #8: 

Thanks, Lachlan (also for your other message). It's all clearer to me now.

I'll post my functions hopefully next week. I'm busy now but I do want to contribute!

Luis Mendo <lmendo>
Mon 18 Jan 2016 01:17:29 AM UTC, comment #7: 

Luis, yes please post your patches or new versions.

Answering your questions:

Yes, Octave does update its behaviour to follow Matlab, if the new behaviour is considered worthwhile.  If users contribute patches, that speeds up the inclusion of the new behaviour.

I don't know if my modified version will be included or not -- I'm just a user like you, not a registered developer.  They're busy getting 4.2 ready now, and it may have to wait until 4.4, but it will probably get there eventually.

If you post your changes here, I'll take a pass to remove issues that I know Rik would comment on (like the coding standards, as far as I understand them) to reduce his workload, and make the inclusion more likely.

Lachlan Andrew <lachlan>
Fri 15 Jan 2016 07:29:17 PM UTC, comment #6: 

Thanks! This seems to work fine.

As for the other functions, I did modify them already to include 'stable'  behaviour, although I didn't extensively test. I can submit them if they may be useful for other people.

Speaking of that, I'm new to Octave so I don't know how this works. Will your modified version be included in a next release of Octave? Would mine as well (after adqeuate testing)? When Matlab changes some function's behaviour, does Octave follow (with a reasonable delay), or does it stick to the old behaviour? What's the role of user contribution regarding this?

Sorry for this many questions! I just want to know how this works, and if my modified functions may help I'll be happy to submit them.

Luis Mendo <lmendo>
Fri 15 Jan 2016 07:00:56 AM UTC, comment #5: 

Thanks, Luis.  The new version fixes that, but may have messed up something else.  Could you give it a try?

Do you want to modify the other functions, or do you want me to?

(file #36043)

Lachlan Andrew <lachlan>
Thu 14 Jan 2016 03:43:33 PM UTC, comment #4: 

@Lachlan

Just to let you know that the third output seems to be given the 'sorted'  result even with the 'stable' input flag. For example,


[aa, bb, cc] = unique('sdassds','sorted')


gives


cc =
   3
   2
   1
   3
   3
   2
   3


when it should give


cc =
     3
     2
     1
     3
     3
     2
     3


Luis Mendo <lmendo>
Thu 14 Jan 2016 02:43:06 PM UTC, comment #3: 

Thanks, Lachlan. I had already written a patched `unique` function with the 'stable' flag, but it only supports the one-output case. Yours seems to be much better. I'll check it and get back to you. Thanks!

Luis Mendo <lmendo>
Thu 14 Jan 2016 06:24:54 AM UTC, comment #2: 

Louis, I've attached a new  unique.m  that implements Matlab's current options and defaults.  As Rik said, this is just .m code; please take a look and let me know if you can see a better way to do things.

If you are about to use 'stable' in any of the other functions, please let me know so that I can prioritise them.  Otherwise, they'll go in the "maybe one day" pile.  Once we're done, I'll submit a patch for Rik to review.

(file #36023)

Lachlan Andrew <lachlan>
Wed 13 Jan 2016 05:15:15 PM UTC, comment #1: 

unique is implemented in an m-file (try 'which unique' inside Octave to find the exact file).  This makes it very easy to change.  There are no immediate plans to work on this, but if you submit a patch I can review it.

Rik <rik5>
Group administrator
Fri 08 Jan 2016 11:56:17 PM UTC, original submission:  

In recent Matlab versions, the `unique` function supports a 'stable' flag. In this mode, it returns the unique elements in the original order, not sorted. There's also a 'sorted' flag, which is the default.

The same happens with set functions: `union`, `intersect`, `setdiff`, `setxor`.

Are there any plans to include this behaviour in Octave?

Thanks for your time!

Luis Mendo <lmendo>

 

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

Attach Files:
   
   
Comment:
   

Attached Files
file #36604:  setxor_comp.m added by lmendo (2KiB - application/octet-stream)
file #36605:  setdiff_comp.m added by lmendo (1KiB - application/octet-stream)
file #36606:  union_comp.m added by lmendo (1KiB - application/octet-stream)
file #36607:  intersect_comp.m added by lmendo (1KiB - application/octet-stream)
file #36302:  union_comp.m added by lmendo (1KiB - application/octet-stream)
file #36303:  intersect_comp.m added by lmendo (1KiB - application/octet-stream)
file #36304:  setdiff_comp.m added by lmendo (1KiB - application/octet-stream)
file #36305:  setxor_comp.m added by lmendo (2KiB - application/octet-stream)
file #36043:  unique.m added by lachlan (9KiB - text/x-objcsrc)
file #36023:  unique.m added by lachlan (9KiB - text/x-objcsrc)

 

Depends on the following items: None found

Items that depend on this one: None found

 

Carbon-Copy List
  • -email is unavailable- added by jwe (Updated the item)
  • -email is unavailable- added by kpc (Posted a comment)
  • -email is unavailable- added by lachlan (Updated the item)
  • -email is unavailable- added by rik5 (Posted a comment)
  • -email is unavailable- added by lmendo (Submitted the item)
  • -email is unavailable- added by lmendo
  •  

    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 group members can vote.

     

    Follow 17 latest changes.

    Date Changed by Updated Field Previous Value => Replaced by
    2019-08-06 rik5 StatusConfirmed Fixed
        Open/ClosedOpen Closed
        Release4.0.0 dev
    2019-01-12 jwe Severity3 - Normal 1 - Wish
        Priority5 - Normal 3 - Low
    2016-03-11 lmendo Attached File- Added setxor_comp.m, #36604
        Attached File- Added setdiff_comp.m, #36605
        Attached File- Added union_comp.m, #36606
        Attached File- Added intersect_comp.m, #36607
    2016-02-10 lmendo Attached File- Added union_comp.m, #36302
        Attached File- Added intersect_comp.m, #36303
        Attached File- Added setdiff_comp.m, #36304
        Attached File- Added setxor_comp.m, #36305
    2016-01-15 lachlan Attached File- Added unique.m, #36043
    2016-01-14 lachlan Attached File- Added unique.m, #36023
    2016-01-13 rik5 StatusNone Confirmed
    2016-01-08 lmendo Carbon-Copy- Added -email is unavailable-

    Back to the top

    Powered by Savane 3.13-f8d8.
    Corresponding source code