[Rd] Consider getNamespaceVersion() returning a numeric_version

[Tim Taylor]

On 17/10/2024 15:53, Prof Brian Ripley wrote:
> On 17/10/2024 13:42, Tim Taylor wrote:
>> I mean the `numeric_version` object not a numeric (double/int). 
>> Basically to protect me from myself I'd prefer not to have to 
>> remember to wrap `getNamespaceVersion()` with `as.package_version()`.
>>
>> I suspect a grep of CRAN may highlight others who are erroneously 
>> comparing character objects rather than a comparison between a 
>> `numeric_version` object and a character.
>
> Perhaps you could do that rather than speculating?  Similarly, try out 
> over CRAN the effect of getNamespaceVersion changing its return type.
>
> It seems to be used in less than 40 CRAN packages, many boilerplate 
> code from a single author and most use the version as a printable 
> character string.  A few are clearly wrong: E.g.
>
> if(getNamespaceVersion("reticulate") >= "1.36.0")
>
> will be false it that package ever reaches "1.100.0".  This is what 
> compareVersion() is for ....

I've raised an issue with the package authors of that particular piece 
of code. I'll try and see what other packages are making similar mistakes.

>
>> On 17/10/2024 13:22, Dirk Eddelbuettel wrote:
>>> On 17 October 2024 at 12:38, Tim Taylor wrote:
>>> | Would R-Core be receptive to having getNamespaceVersion() return a
>>> | numeric_version object instead of a named character?
>>>
>>> Is this good enough? What's your actual issue a 'numeric' would 
>>> address?
>>>
>>>     > as.package_version(getNamespaceVersion("base")) < "4.5.0"
>>>     [1] TRUE
>>>     >
>>>     > as.package_version(getNamespaceVersion("Rcpp")) > "1.0.11"
>>>     [1] TRUE
>>>     > as.package_version(getNamespaceVersion("Rcpp")) > "1.0.14"
>>>     [1] FALSE
>
> There are differences, e.g.
>
> > (z <- getNamespaceVersion("MASS"))
>  version
> "7.3-61"
> > (zz <- as.package_version(z))
> [1] ‘7.3.61’
> > as.character(zz)
> [1] "7.3.61"
>
> and some uses need the first.  That makes changing the return value 
> too disruptive.
>
> If the issue is only comparison, getNamespaceVersion's return value 
> could be given a class and an Ops group method, but the existence of 
> compareVersion() makes that less compelling.

Yes the issue for me was with a comparison. I think the additional class 
and Ops method could be worthwhile to prevent others from making a 
similar mistake to myself. That said I do appreciate it does add more 
code when there are already alternatives available. If you'd be 
receptive I'd be happy to submit a patch in this regards.

Many thanks

Tim