bugGNU Octave - Bugs: bug #55525, verLessThan() is unimplemented

 
 

bug #55525: verLessThan() is unimplemented

Submitter:  Andrew Janke <apjanke>
Submitted:  Sun 20 Jan 2019 02:02:12 PM UTC
   
 
Category:  Octave Function Severity:  3 - Normal
Priority:  5 - Normal Item Group:  Feature Request
Status:  Fixed Assigned to:  None
Originator Name:  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

Wed 23 Jan 2019 05:36:28 PM UTC, comment #10: 

I appreciate the appreciation! I'll keep notes and hopefully this will get easier over time.

Andrew Janke <apjanke>
Wed 23 Jan 2019 05:10:08 PM UTC, comment #9: 

I made some additional improvements.  See cset https://hg.savannah.gnu.org/hgweb/octave/rev/eaebee6d4f56.  The commit message was:


* verLessThan.m: Use standard documentation phrasing for a function which
returns true/false.  Use @qcode instead of @code for strings in documentation.
Use standard variable name "retval" for output.  Validate number of input
arguments to function.  Use rows() rather than "size (..., 1)" for clarity.
Capitalize input variable names in error() strings.  Do not end error strings
with a period.  Use single quotes for strings containing double quotes to avoid
the unclarity of doubling '"'.  Add input validation BIST tests for number of
arguments.

* __unimplemented__.m: Remove verLessThan from list of unimplemented functions.


Thanks Andrew for coding this up.  I know there is a steep learning curve associated with programming for Octave core, so I want to acknowledge that and say that it does get easier.

Marking as fixed and closing report.

Rik <rik5>
Group administrator
Wed 23 Jan 2019 10:22:22 AM UTC, comment #8: 

Sounds good. Thank you!

I'll keep those style items in mind for next time.

Andrew Janke <apjanke>
Wed 23 Jan 2019 10:12:13 AM UTC, comment #7: 

@Andrew: I pushed the patch to default. Here are the modifications I made:

  • doc: Enclosed example in @group environment as we usually do for better looking result
  • changed variable name ix to idx, as the latter is widely used in Octave code base for index variables
  • added white space after "!" operator
  • removed intermediate variable pkg_ver
  • tests: used the complete expected error string
  • Added verLessThan to the list of new functions (NEWS), to the list of functions in the miscellaneous directory (corresponding module.mk) and to the manual (system.txi)


http://hg.savannah.gnu.org/hgweb/octave/rev/0f187a956d47

Pantxo Diribarne <pantxo>
Group Member
Tue 22 Jan 2019 02:14:41 AM UTC, comment #6: 

This should go on the development branch when ready for commit.

Rik <rik5>
Group administrator
Mon 21 Jan 2019 11:03:47 PM UTC, comment #5: 

I default to using ismember() because it's polymorphic and more concise. And for a function like this that isn't going to be called inside a loop, I wouldn't think that that performance difference would matter.

Here's a new revision that switches to strcmpi and I think addresses your remaining concerns: file #46063:  add-verlessthan-v4.patch

Andrew Janke <apjanke>
Mon 21 Jan 2019 10:07:38 AM UTC, comment #4: 

@Andrew: Why use ismember (m-code) when you can use strcmp (and I think we should probably use strcmpi) which is a builtin and thus probably faster function? Also, two of the points I mentioned were not addressed:

  • error messages should be prepended with the function name: "error ("verLessThan: ...")"
  • if conditions should be between parents: "if (condition)".
Pantxo Diribarne <pantxo>
Group Member
Sun 20 Jan 2019 07:34:53 PM UTC, comment #3: 

Thanks; I wasn't aware of compare_versions.

Here's a revision that switches it to use compare_versions and addresses your other concerns: file #46048:  add-verlessthan-v3.patch

Andrew Janke <apjanke>
Sun 20 Jan 2019 05:20:50 PM UTC, comment #2: 

We have an Octave specific function "compare_versions". It would probably make sense to use it for the version string comparison since it handles other characters like "+".

Other comments:

  • Since loops are very slow in Octave we try to vectorize codes as much as possible. The code could be rewritten to something like


v = ver ();
idx = strcmp ({v.Name}, package);
if (! any (idx))
  error ("verLessThan(): PACKAGE ""%s"" is not installed.", package);
endif

out = compare_versions ({v.Version}{idx}, version, "<");


  • Conditions in "if" or "while" statements should be surrounded with parenthesis
  • error/warning strings are prepended with the function name.
  • There is a typo, "datenum" should be replaced by "verLessThan" in the doc string.


I changed the item group, since "Matlab Compatibility" is reserved for already implemented features that behave differently in Octave and Matlab.

Pantxo Diribarne <pantxo>
Group Member
Sun 20 Jan 2019 02:26:55 PM UTC, comment #1: 

Here's a revision with more Octave-y names in the doco and variables: file #46040:  add-verlessthan-v2.patch

Andrew Janke <apjanke>
Sun 20 Jan 2019 02:02:12 PM UTC, original submission:  

Octave doesn't provide a verLessThan() function like Matlab does.

Here's a patch to implement one. This lets you test Octave and toolbox versions and write version-specific code or dynamically assert requirements.

Andrew Janke <apjanke>

 

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

Attach Files:
   
   
Comment:
   

Attached Files
file #46063:  add-verlessthan-v4.patch added by apjanke (3KiB - application/octet-stream)
file #46048:  add-verlessthan-v3.patch added by apjanke (3KiB - application/octet-stream)
file #46047:  add-verlessthan-v3.patch added by apjanke (3KiB - application/octet-stream)
file #46040:  add-verlessthan-v2.patch added by apjanke (3KiB - application/octet-stream)
file #46039:  add-verlessthan.patch added by apjanke (3KiB - application/octet-stream)

 

Depends on the following items: None found

Items that depend on this one: None found

 

Carbon-Copy List
  • -email is unavailable- added by rik5 (Posted a comment)
  • -email is unavailable- added by pantxo (Posted a comment)
  • -email is unavailable- added by apjanke (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 10 latest changes.

    Date Changed by Updated Field Previous Value => Replaced by
    2019-01-23 rik5 StatusReady For Test Fixed
        Open/ClosedOpen Closed
    2019-01-23 pantxo StatusPatch Submitted Ready For Test
    2019-01-21 apjanke Attached File- Added add-verlessthan-v4.patch, #46063
    2019-01-21 pantxo StatusNone Patch Submitted
    2019-01-20 apjanke Attached File- Added add-verlessthan-v3.patch, #46048
    2019-01-20 apjanke Attached File- Added add-verlessthan-v3.patch, #46047
    2019-01-20 pantxo Item GroupMatlab Compatibility Feature Request
    2019-01-20 apjanke Attached File- Added add-verlessthan-v2.patch, #46040
    2019-01-20 apjanke Attached File- Added add-verlessthan.patch, #46039

    Back to the top

    Powered by Savane 3.13-4448.
    Corresponding source code