bugGNU Octave - Bugs: bug #56065, New function "contains"

 
 

bug #56065: New function "contains"

Submitter:  Marc Dirix <dirixmjm>
Submitted:  Wed 03 Apr 2019 11:16:21 AM UTC
   
 
Category:  Octave Function Severity:  1 - Wish
Priority:  3 - Low Item Group:  Feature Request
Status:  Patch Submitted Assigned to:  None
Originator Name:  contains Open/Closed:  * Open
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 20 Oct 2020 01:58:18 PM UTC, comment #6: 

Thank you for your contribution and sorry it took so long until a review.

I haven't actually tested the new function. But a few preliminary comments to the latest patch file #48549 (some or most of them are nitpicking. Sorry for that.):
- The file has the wrong name (in the patch).
- The copyright says 2008-2020. But that file isn't from 2008.
- There are some spelling and grammar errors in the doc string.
- We usually have the input check as a first block before the "actual functionality".
- Also the input check seems to be incomplete. Instead of "iscell", you should probably use "iscellstr". The third input argument is ignored.
- Use parenthesis around if-conditions.
- Prefer to use double quoted strings.
- Use spaces between function name and opening parenthesis (in BISTs and example in docstring).
- Add ids to errors. (That one is new.)
- Use "!" not "~" for negation.
- Add new file to build system (module.mk).
- Add note about new file to NEWS file.

I started to correct those issues when I realized that the BISTs and examples are copies from Matlab's documentation for that function. I'm not sure it is legal to use those.

It would be nice if you could change those to less problematic examples.

Please, use the attached work-in-progress patch if possible.

(file #50029)

Markus Mützel <mmuetzel>
Group administrator
Mon 16 Mar 2020 03:08:07 PM UTC, comment #5: 

If changes are required to merge, that's fine with me.

Marshall <marsian>
Wed 04 Mar 2020 08:48:20 AM UTC, comment #4: 

I tried to write the file in a way that we able to merge to our system Is this okay?

Ahmed ElShreif <shikoz>
Tue 05 Nov 2019 06:47:58 PM UTC, comment #3: 

Here's a function I wrote to meet matlab compatibility


%contains returns true if the pattern is found in the string(s).
%
%  Usage:
%   b = contains(str,pattern)
%   b = contains(str,pattern,'IgnoreCase',ignorecase)
%
%  str and pattern may be a character array or a cell array of strings and do not
%   have to be the same size.
%  b is the same size as str and returns true for each element in str that
%   matches any of the pattern strings specified.
%  if ignorecase is true, then the match is case insensitive (default is false)
%
%   Examples
%       str = 'data.tar.gz';
%       P = 'tar';
%       contains(str,P)                   returns  1
%
%       str = {'abstracts.docx','data.tar.gz'};
%       P = 'tar';
%       contains(str,P)                   returns  [0 1]
%
%       str = 'data.tar.gz';
%       P = {'docx','tar'};
%       contains(str,P)                   returns  1
%
%       str ={'DATA.TAR.GZ','SUMMARY.PPT'};
%       P = 'tar';
%       contains(str,P,'IgnoreCase',true) returns  [1 0]
function [b] = contains(str,pattern,~,ignorecase=false)
    if ignorecase
        str=lower(str);
        pattern=lower(pattern);
    endif
    if ischar(str)
        if ischar(pattern)
            b=~isempty(strfind(str,pattern));
        elseif iscell(pattern)
            b=any(cellfun(@(p)~isempty(p),strfind(str,pattern)));
        else
            error('pattern must be a char array or cell array of strings');
        endif
    elseif iscell(str)
        if isstr(pattern)
            b=cellfun(@(s)~isempty(s),strfind(str,pattern));
        elseif iscell(pattern)
            b=cellfun(@(s)any(cellfun(@(p)~isempty(p),strfind(s,pattern))),str);
        else
            error('pattern must be a char array or cell array of strings');
        endif
    else
        error('str must be a char array or cell array of strings');
    endif
endfunction

%!assert(contains('data.tar.gz','tar'),true)
%!assert(contains('peppers, onions, and mushrooms','onion'),true);
%!assert(contains('peppers, onions, and mushrooms','pineapples'),false);

%!assert(contains({'abstracts.docx','data.tar.gz'},'tar'),logical([0 1]))
%!assert(contains({'abstracts.docx','data.tar.gz'},'zip'),logical([0 0]))

%!assert(contains('data.tar.gz',{'docx','tar'}),true)
%!assert(contains('data.tar.gz',{'7z','zip'}),false)

%!test
%! str = {'Mary Ann Jones','Christopher Matthew Burns','John Paul Smith'};
%! pattern = {'Ann','Paul'};
%! assert(contains(str,pattern),logical([1 0 1]))
%! assert(str(contains(str,pattern)),{'Mary Ann Jones','John Paul Smith'})

%!test
%! str ={'DATA.TAR.GZ','SUMMARY.PPT'};
%! P = 'tar';
%! assert(contains(str,P,'IgnoreCase',true),logical([1 0]))
%! assert(contains(str,P,'IgnoreCase',false),logical([0 0]))


Marshall <marsian>
Fri 01 Nov 2019 10:24:08 PM UTC, comment #2: 

The function here doesn't work in the cases as specified by matlab. The following are examples taken from Matlab help/documentation


        STR = 'data.tar.gz';
        P = 'tar';
        assert(contains(STR,P),true)

        chr = 'peppers, onions, and mushrooms';
        assert(contains(chr,'onion'),true);
        assert(contains(chr,'pineapples'),false);

        STR = {'abstracts.docx','data.tar.gz'};
        P = 'tar';
        assert(contains(STR,P),logical([0 1]))

        STR = 'data.tar.gz';
        P = {'docx','tar'};
        assert(contains(STR,P),true)

        str = {'Mary Ann Jones','Christopher Matthew Burns','John Paul Smith'};
        pattern = {'Ann','Paul'};
        assert(contains(str,pattern),logical([1 0 1]))
        assert(str(contains(str,pattern)),{'Mary Ann Jones','John Paul Smith'})

        STR ={'DATA.TAR.GZ','SUMMARY.PPT'};
        P = 'tar';
        assert(contains(STR,P,'IgnoreCase',true),logical([1 0]))

The code submitted here passes the first two cases, but not the third or later. The third assertion should be a fundamental test that is not passed, whereas the later assertions are 'advanced' features in order of what I think is important (I use the fourth case frequently, but none of the later cases).

Marshall <marsian>
Wed 03 Apr 2019 01:25:36 PM UTC, comment #1: 

Thank you a lot for your contribution.  I lowered the priority as it is a new function and might be included in the development version once your patch is ready.

To increase your chances to add your implementation to Octave, please take a look at those various examples in

https://hg.savannah.gnu.org/hgweb/octave/file/30e9204de313/scripts

and add your copyright and the GPL text, some appropriate documentation, and test cases according to the Matlab documentation. But please not copy&past from Matlab because of copyright violations.

Kai Torben Ohlhus <siko1056>
Group Member
Wed 03 Apr 2019 11:16:21 AM UTC, original submission:  

I have implemented an initial version of the contains function which is missing from Matlab.

Marc Dirix <dirixmjm>

 

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

Attach Files:
   
   
Comment:
   

Attached Files
file #50029:  bug56065_contains_v2.patch added by mmuetzel (6KiB - application/octet-stream)
file #48549:  mypatch.patch added by shikoz (4KiB - application/octet-stream)
file #46709:  contains.m added by dirixmjm (912B - text/plain)

 

Depends on the following items: None found

Digest:
   bug dependencies.

 

Carbon-Copy List
  • -email is unavailable- added by mmuetzel (Updated the item)
  • -email is unavailable- added by shikoz (Updated the item)
  • -email is unavailable- added by marsian (Posted a comment)
  • -email is unavailable- added by siko1056 (Posted a comment)
  • -email is unavailable- added by dirixmjm (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 group members can vote.

     

    Follow 12 latest changes.

    Date Changed by Updated Field Previous Value => Replaced by
    2020-10-20 mmuetzel Attached File- Added bug56065_contains_v2.patch, #50029
    2020-03-04 shikoz Attached File- Added mypatch.patch, #48549
    2019-10-11 mtmiller Dependencies- bugs #57045 is dependent
    2019-04-03 mtmiller Severity2 - Minor 1 - Wish
    2019-04-03 rik5 Carbon-CopyRemoved 72865 -
    2019-04-03 rik5 Item GroupMatlab Compatibility Feature Request
    2019-04-03 siko1056 Severity3 - Normal 2 - Minor
        Priority5 - Normal 3 - Low
        StatusNone Patch Submitted
        Release5.1.0 dev
        Summarymatlab function &quot;contains&quot; missing New function "contains"
    2019-04-03 dirixmjm Attached File- Added contains.m, #46709

    Back to the top

    Powered by Savane 3.13-758e.
    Corresponding source code