Question regarding .make_numeric_version with non-character input

Andrea Gilardi via R-devel writes:
Thanks: should be fixed now in the trunk.

Best
-k
Thank you very much Dirk for your kind words and for confirming the bug. 
Next week I will open a new issue on Bugzilla adding the related patch.
Kind regards
Andrea
On 29/03/2024 20:14, Dirk Eddelbuettel wrote:
On 29 March 2024 at 17:56, Andrea Gilardi via R-devel wrote:
| Dear all,
|
| I have a question regarding the R-devel version of .make_numeric_version() function. As far as I can understand, the current code (https://github.com/wch/r-source/blob/66b91578dfc85140968f07dd4e72d8cb8a54f4c6/src/library/base/R/version.R#L50-L56) runs the following steps in case of non-character input:
|
| 1. It creates a message named msg using gettextf.
| 2. Such object is then passed to stop(msg) or warning(msg) according to the following condition
|
| tolower(Sys.getenv("_R_CHECK_STOP_ON_INVALID_NUMERIC_VERSION_INPUTS_") != "false")
|
| However, I don't understand the previous code since the output of Sys.getenv("_R_CHECK_STOP_ON_INVALID_NUMERIC_VERSION_INPUTS_") != "false" is just a boolean value and tolower() will just return "true" or "false". Maybe the intended code is tolower(Sys.getenv("_R_CHECK_STOP_ON_INVALID_NUMERIC_VERSION_INPUTS_")) != "false" ? Or am I missing something?

Yes, agreed -- good catch.  In full, the code is (removing leading
whitespace, and putting it back onto single lines)

msg <- gettextf("invalid non-character version specification 'x' (type: %s)", typeof(x))
if(tolower(Sys.getenv("_R_CHECK_STOP_ON_INVALID_NUMERIC_VERSION_INPUTS_") != "false"))
stop(msg, domain = NA)
else
warning(msg, domain = NA, immediate. = TRUE)

where msg is constant (but reflecting language settings via standard i18n)
and as you not the parentheses appear wrong.  What was intended is likely

msg <- gettextf("invalid non-character version specification 'x' (type: %s)", typeof(x))
if(tolower(Sys.getenv("_R_CHECK_STOP_ON_INVALID_NUMERIC_VERSION_INPUTS_")) != "false")
stop(msg, domain = NA)
else
warning(msg, domain = NA, immediate. = TRUE)

If you use bugzilla before and have a handle, maybe file a bug report with
this as patch at https://bugs.r-project.org/

Dirk

______________________________________________
R-devel at r-project.org mailing list
https://stat.ethz.ch/mailman/listinfo/r-devel
Hi Kurt,

Is it intended that numeric_version() returns an error by default on 
non-character input in R 4.4.0? It seems that I can turn this into a 
warning by setting 
_R_CHECK_STOP_ON_INVALID_NUMERIC_VERSION_INPUTS_=false but I don't seem 
to be able to find any of this mentioned in the NEWS file.

Thanks,

H.
Andrea Gilardi via R-devel writes:
Thanks: should be fixed now in the trunk.

Best
-k

Thank you very much Dirk for your kind words and for confirming the bug.
Next week I will open a new issue on Bugzilla adding the related patch.
Kind regards
Andrea
On 29/03/2024 20:14, Dirk Eddelbuettel wrote:
On 29 March 2024 at 17:56, Andrea Gilardi via R-devel wrote:
| Dear all,
|
| I have a question regarding the R-devel version of .make_numeric_version() function. As far as I can understand, the current code (https://github.com/wch/r-source/blob/66b91578dfc85140968f07dd4e72d8cb8a54f4c6/src/library/base/R/version.R#L50-L56) runs the following steps in case of non-character input:
|
| 1. It creates a message named msg using gettextf.
| 2. Such object is then passed to stop(msg) or warning(msg) according to the following condition
|
| tolower(Sys.getenv("_R_CHECK_STOP_ON_INVALID_NUMERIC_VERSION_INPUTS_") != "false")
|
| However, I don't understand the previous code since the output of Sys.getenv("_R_CHECK_STOP_ON_INVALID_NUMERIC_VERSION_INPUTS_") != "false" is just a boolean value and tolower() will just return "true" or "false". Maybe the intended code is tolower(Sys.getenv("_R_CHECK_STOP_ON_INVALID_NUMERIC_VERSION_INPUTS_")) != "false" ? Or am I missing something?

Yes, agreed -- good catch.  In full, the code is (removing leading
whitespace, and putting it back onto single lines)

msg <- gettextf("invalid non-character version specification 'x' (type: %s)", typeof(x))
if(tolower(Sys.getenv("_R_CHECK_STOP_ON_INVALID_NUMERIC_VERSION_INPUTS_") != "false"))
stop(msg, domain = NA)
else
warning(msg, domain = NA, immediate. = TRUE)

where msg is constant (but reflecting language settings via standard i18n)
and as you not the parentheses appear wrong.  What was intended is likely

msg <- gettextf("invalid non-character version specification 'x' (type: %s)", typeof(x))
if(tolower(Sys.getenv("_R_CHECK_STOP_ON_INVALID_NUMERIC_VERSION_INPUTS_")) != "false")
stop(msg, domain = NA)
else
warning(msg, domain = NA, immediate. = TRUE)

If you use bugzilla before and have a handle, maybe file a bug report with
this as patch athttps://bugs.r-project.org/

Dirk

______________________________________________
R-devel at r-project.org  mailing list
https://stat.ethz.ch/mailman/listinfo/r-devel
______________________________________________
R-devel at r-project.org  mailing list
https://stat.ethz.ch/mailman/listinfo/r-devel

Herv? Pag?s

Bioconductor Core Team
hpages.on.github at gmail.com

	[[alternative HTML version deleted]]
Herv? Pag?s writes:
Hi Kurt,
Is it intended that numeric_version() returns an error by default on
non-character input in R 4.4.0? 
Dear Herve, yes, that's the intention.
It seems that I can turn this into a warning by setting
_R_CHECK_STOP_ON_INVALID_NUMERIC_VERSION_INPUTS_=false but I don't
seem to be able to find any of this mentioned in the NEWS file.
That's what I added for smoothing the transition: it will be removed
from the trunk shortly.

Best
-k
Thanks,
H.
On 4/1/24 05:28, Kurt Hornik wrote:
                        Andrea Gilardi via R-devel writes:
    Thanks: should be fixed now in the trunk.
    Best
    -k
        Thank you very much Dirk for your kind words and for confirming the bug. 
        Next week I will open a new issue on Bugzilla adding the related patch.
        Kind regards
        Andrea
        On 29/03/2024 20:14, Dirk Eddelbuettel wrote:
            On 29 March 2024 at 17:56, Andrea Gilardi via R-devel wrote:
            | Dear all,
            |
            | I have a question regarding the R-devel version of .make_numeric_version() function. As far as I can understand, the current code (https://github.com/wch/r-source/blob/66b91578dfc85140968f07dd4e72d8cb8a54f4c6/src/library/base/R/version.R#L50-L56) runs the following steps in case of non-character input:
            |
            | 1. It creates a message named msg using gettextf.
            | 2. Such object is then passed to stop(msg) or warning(msg) according to the following condition
            |
            | tolower(Sys.getenv("_R_CHECK_STOP_ON_INVALID_NUMERIC_VERSION_INPUTS_") != "false")
            |
            | However, I don't understand the previous code since the output of Sys.getenv("_R_CHECK_STOP_ON_INVALID_NUMERIC_VERSION_INPUTS_") != "false" is just a boolean value and tolower() will just return "true" or "false". Maybe the intended code is tolower(Sys.getenv("_R_CHECK_STOP_ON_INVALID_NUMERIC_VERSION_INPUTS_")) != "false" ? Or am I missing something?
            Yes, agreed -- good catch.  In full, the code is (removing leading
            whitespace, and putting it back onto single lines)
            msg <- gettextf("invalid non-character version specification 'x' (type: %s)", typeof(x))
            if(tolower(Sys.getenv("_R_CHECK_STOP_ON_INVALID_NUMERIC_VERSION_INPUTS_") != "false"))
            stop(msg, domain = NA)
            else
            warning(msg, domain = NA, immediate. = TRUE)
            where msg is constant (but reflecting language settings via standard i18n)
            and as you not the parentheses appear wrong.  What was intended is likely
            msg <- gettextf("invalid non-character version specification 'x' (type: %s)", typeof(x))
            if(tolower(Sys.getenv("_R_CHECK_STOP_ON_INVALID_NUMERIC_VERSION_INPUTS_")) != "false")
            stop(msg, domain = NA)
            else
            warning(msg, domain = NA, immediate. = TRUE)
            If you use bugzilla before and have a handle, maybe file a bug report with
            this as patch at https://bugs.r-project.org/
            Dirk

        ______________________________________________
        R-devel at r-project.org mailing list
        https://stat.ethz.ch/mailman/listinfo/r-devel

    ______________________________________________
    R-devel at r-project.org mailing list
    https://stat.ethz.ch/mailman/listinfo/r-devel

-- 
Herv? Pag?s
Bioconductor Core Team
hpages.on.github at gmail.com

Herv? Pag?s writes:
Hi Kurt,
Is it intended that numeric_version() returns an error by default on
non-character input in R 4.4.0?
Dear Herve, yes, that's the intention.

It seems that I can turn this into a warning by setting
_R_CHECK_STOP_ON_INVALID_NUMERIC_VERSION_INPUTS_=false but I don't
seem to be able to find any of this mentioned in the NEWS file.
That's what I added for smoothing the transition: it will be removed
from the trunk shortly.
Thanks for clarifying.? Could this be documented in the NEWS file? This 
is a breaking change (it breaks a couple of Bioconductor packages) and 
people are not going to set this environment variable if they are not 
aware of it.

Thanks again,

H.
Best
-k

Thanks,
H.
On 4/1/24 05:28, Kurt Hornik wrote:
                         Andrea Gilardi via R-devel writes:

     Thanks: should be fixed now in the trunk.

     Best
     -k
         Thank you very much Dirk for your kind words and for confirming the bug.
         Next week I will open a new issue on Bugzilla adding the related patch.

         Kind regards

         Andrea

         On 29/03/2024 20:14, Dirk Eddelbuettel wrote:

             On 29 March 2024 at 17:56, Andrea Gilardi via R-devel wrote:
             | Dear all,
             |
             | I have a question regarding the R-devel version of .make_numeric_version() function. As far as I can understand, the current code (https://github.com/wch/r-source/blob/66b91578dfc85140968f07dd4e72d8cb8a54f4c6/src/library/base/R/version.R#L50-L56) runs the following steps in case of non-character input:
             |
             | 1. It creates a message named msg using gettextf.
             | 2. Such object is then passed to stop(msg) or warning(msg) according to the following condition
             |
             | tolower(Sys.getenv("_R_CHECK_STOP_ON_INVALID_NUMERIC_VERSION_INPUTS_") != "false")
             |
             | However, I don't understand the previous code since the output of Sys.getenv("_R_CHECK_STOP_ON_INVALID_NUMERIC_VERSION_INPUTS_") != "false" is just a boolean value and tolower() will just return "true" or "false". Maybe the intended code is tolower(Sys.getenv("_R_CHECK_STOP_ON_INVALID_NUMERIC_VERSION_INPUTS_")) != "false" ? Or am I missing something?

             Yes, agreed -- good catch.  In full, the code is (removing leading
             whitespace, and putting it back onto single lines)

             msg <- gettextf("invalid non-character version specification 'x' (type: %s)", typeof(x))
             if(tolower(Sys.getenv("_R_CHECK_STOP_ON_INVALID_NUMERIC_VERSION_INPUTS_") != "false"))
             stop(msg, domain = NA)
             else
             warning(msg, domain = NA, immediate. = TRUE)

             where msg is constant (but reflecting language settings via standard i18n)
             and as you not the parentheses appear wrong.  What was intended is likely

             msg <- gettextf("invalid non-character version specification 'x' (type: %s)", typeof(x))
             if(tolower(Sys.getenv("_R_CHECK_STOP_ON_INVALID_NUMERIC_VERSION_INPUTS_")) != "false")
             stop(msg, domain = NA)
             else
             warning(msg, domain = NA, immediate. = TRUE)

             If you use bugzilla before and have a handle, maybe file a bug report with
             this as patch athttps://bugs.r-project.org/

             Dirk
         ______________________________________________
         R-devel at r-project.org  mailing list
         https://stat.ethz.ch/mailman/listinfo/r-devel

     ______________________________________________
     R-devel at r-project.org  mailing list
     https://stat.ethz.ch/mailman/listinfo/r-devel

-- 
Herv? Pag?s
Bioconductor Core Team
hpages.on.github at gmail.com

Herv? Pag?s

Bioconductor Core Team
hpages.on.github at gmail.com

	[[alternative HTML version deleted]]
Hi Kurt,
| >>>>> Herv? Pag?s writes:
| 
| > Hi Kurt,
| > Is it intended that numeric_version() returns an error by default on
| > non-character input in R 4.4.0? 
| 
| Dear Herve, yes, that's the intention.
| 
| > It seems that I can turn this into a warning by setting
| > _R_CHECK_STOP_ON_INVALID_NUMERIC_VERSION_INPUTS_=false but I don't
| > seem to be able to find any of this mentioned in the NEWS file.
| 
| That's what I added for smoothing the transition: it will be removed
| from the trunk shortly.

I would actually be nice to have a more robust variant for non-CRAN
versions. For example I just had to do a local hack to be able to use what
the QuantLib 'rc' 1.34-rc reported (when I then used to R facilities to
condition code and tests on whether I was dealing with code before or after
an API transition).  So as a wishlist: could you envision an extension to
package_version() casting that, say, removes all [a-zA-Z]+ first (if opted
into) ?

Dirk
dirk.eddelbuettel.com | @eddelbuettel | edd at debian.org
Dirk Eddelbuettel writes:
Hi Kurt,
On 25 April 2024 at 08:07, Kurt Hornik wrote:
| >>>>> Herv? Pag?s writes:
| 
| > Hi Kurt,
| > Is it intended that numeric_version() returns an error by default on
| > non-character input in R 4.4.0? 
| 
| Dear Herve, yes, that's the intention.
| 
| > It seems that I can turn this into a warning by setting
| > _R_CHECK_STOP_ON_INVALID_NUMERIC_VERSION_INPUTS_=false but I don't
| > seem to be able to find any of this mentioned in the NEWS file.
| 
| That's what I added for smoothing the transition: it will be removed
| from the trunk shortly.
I would actually be nice to have a more robust variant for non-CRAN
versions. For example I just had to do a local hack to be able to use
what the QuantLib 'rc' 1.34-rc reported (when I then used to R
facilities to condition code and tests on whether I was dealing with
code before or after an API transition).  So as a wishlist: could you
envision an extension to package_version() casting that, say, removes
all [a-zA-Z]+ first (if opted into) ?
Well, if I could turn back time and start again, I'd implement package
versions in the Debian way, and not numeric only.  As you know, the
current approach does not conveniently allow for handling binary
revisions or NMUs.

Currently, package_version extends numeric_version, but in principle
that could be changed: we would of course have to ensure that we go on
using numeric-only package versions for source packages so that older
versions of R can handle these.

One could in principle also enhance the 'strict' argument so that
e.g. strict = NA says drop all non-numeric non-sep parts, but it would
be better to first figure out whether it wouldn't be better to make
things work for non-numeric version components too :-)

Best
-k
Dirk
-- 
dirk.eddelbuettel.com | @eddelbuettel | edd at debian.org
Herv? Pag?s writes:
On 4/24/24 23:07, Kurt Hornik wrote:
Herv? Pag?s writes:
Hi Kurt,
Is it intended that numeric_version() returns an error by default on
non-character input in R 4.4.0?
Dear Herve, yes, that's the intention.

It seems that I can turn this into a warning by setting
_R_CHECK_STOP_ON_INVALID_NUMERIC_VERSION_INPUTS_=false but I don't
seem to be able to find any of this mentioned in the NEWS file.
That's what I added for smoothing the transition: it will be removed
from the trunk shortly.
Thanks for clarifying.? Could this be documented in the NEWS file? This 
is a breaking change (it breaks a couple of Bioconductor packages) and 
people are not going to set this environment variable if they are not 
aware of it.
Sure, I'll look into adding something.  (Too late for 4.4.0, of course.)

Best
-k
Thanks again,
H.
Best
-k

Thanks,
H.
On 4/1/24 05:28, Kurt Hornik wrote:
Andrea Gilardi via R-devel writes:

Thanks: should be fixed now in the trunk.

Best
-k
Thank you very much Dirk for your kind words and for confirming the bug.
Next week I will open a new issue on Bugzilla adding the related patch.

Kind regards

Andrea

On 29/03/2024 20:14, Dirk Eddelbuettel wrote:

On 29 March 2024 at 17:56, Andrea Gilardi via R-devel wrote:
| Dear all,
|
| I have a question regarding the R-devel version of .make_numeric_version() function. As far as I can understand, the current code (https://github.com/wch/r-source/blob/66b91578dfc85140968f07dd4e72d8cb8a54f4c6/src/library/base/R/version.R#L50-L56) runs the following steps in case of non-character input:
|
| 1. It creates a message named msg using gettextf.
| 2. Such object is then passed to stop(msg) or warning(msg) according to the following condition
|
| tolower(Sys.getenv("_R_CHECK_STOP_ON_INVALID_NUMERIC_VERSION_INPUTS_") != "false")
|
| However, I don't understand the previous code since the output of Sys.getenv("_R_CHECK_STOP_ON_INVALID_NUMERIC_VERSION_INPUTS_") != "false" is just a boolean value and tolower() will just return "true" or "false". Maybe the intended code is tolower(Sys.getenv("_R_CHECK_STOP_ON_INVALID_NUMERIC_VERSION_INPUTS_")) != "false" ? Or am I missing something?

Yes, agreed -- good catch.  In full, the code is (removing leading
whitespace, and putting it back onto single lines)

msg <- gettextf("invalid non-character version specification 'x' (type: %s)", typeof(x))
if(tolower(Sys.getenv("_R_CHECK_STOP_ON_INVALID_NUMERIC_VERSION_INPUTS_") != "false"))
stop(msg, domain = NA)
else
warning(msg, domain = NA, immediate. = TRUE)

where msg is constant (but reflecting language settings via standard i18n)
and as you not the parentheses appear wrong.  What was intended is likely

msg <- gettextf("invalid non-character version specification 'x' (type: %s)", typeof(x))
if(tolower(Sys.getenv("_R_CHECK_STOP_ON_INVALID_NUMERIC_VERSION_INPUTS_")) != "false")
stop(msg, domain = NA)
else
warning(msg, domain = NA, immediate. = TRUE)

If you use bugzilla before and have a handle, maybe file a bug report with
this as patch athttps://bugs.r-project.org/

Dirk
______________________________________________
R-devel at r-project.org  mailing list
https://stat.ethz.ch/mailman/listinfo/r-devel

______________________________________________
R-devel at r-project.org  mailing list
https://stat.ethz.ch/mailman/listinfo/r-devel

-- 
Herv? Pag?s
Bioconductor Core Team
hpages.on.github at gmail.com
-- 
Herv? Pag?s
Bioconductor Core Team
hpages.on.github at gmail.com
	[[alternative HTML version deleted]]

______________________________________________
R-devel at r-project.org mailing list
https://stat.ethz.ch/mailman/listinfo/r-devel
...
Sure, I'll look into adding something.  (Too late for 4.4.0, of course.)

Best
-k
Great. Thanks!

H.
Herv? Pag?s

Bioconductor Core Team
hpages.on.github at gmail.com

	[[alternative HTML version deleted]]