Re: [R-pkg-devel] [External] [External] RcmdrPlugin.HH_1.1-48.tar.gz

2024-03-07 Thread Martin Maechler
> Duncan Murdoch 
> on Thu, 7 Mar 2024 05:08:40 -0500 writes:

> On 07/03/2024 4:16 a.m., Ivan Krylov wrote:
>> On Wed, 6 Mar 2024 13:46:55 -0500 Duncan Murdoch
>>  wrote:
>> 
>>> is this just a more or less harmless error, thinking
>>> that the dot needs escaping
>> 
>> I think it's this one. You are absolutely right that the
>> dot doesn't need escaping in either TRE (which is what's
>> used inside exportPattern) or PCRE. In PRCE, this regular
>> expression would have worked as intended:
>> 
>> # We do match backslashes by mistake.  grepl('[\\.]',
>> '\\') # [1] TRUE
>> 
>> # In PCRE, this wouldn't have been a mistake.
>> grepl('[\\.]', c('\\', '.'), perl = TRUE) # [1] FALSE
>> TRUE
>> 

> Thanks, I didn't realize that escaping in PCRE was
> optional.

> So the default exportPattern line could be

>exportPattern("^[^.]")

> and it would work even if things were changed so that PCRE
> was used instead of TRE.

> Duncan Murdoch

Yes, thank you, Duncan!
.. I had started changing to this much easier pattern already
before reading on ...  -->  in R-devel; now also with "doc"s (NEWS.Rd, 
R-exts.texi)

Martin

__
R-package-devel@r-project.org mailing list
https://stat.ethz.ch/mailman/listinfo/r-package-devel


Re: [R-pkg-devel] [External] [External] RcmdrPlugin.HH_1.1-48.tar.gz

2024-03-07 Thread Serguei Sokol

Le 07/03/2024 à 11:08, Duncan Murdoch a écrit :

On 07/03/2024 4:16 a.m., Ivan Krylov wrote:

On Wed, 6 Mar 2024 13:46:55 -0500
Duncan Murdoch  wrote:


is this just a more or less harmless error, thinking that
the dot needs escaping


I think it's this one. You are absolutely right that the dot doesn't
need escaping in either TRE (which is what's used inside exportPattern)
or PCRE. In PRCE, this regular expression would have worked as intended:

# We do match backslashes by mistake.
grepl('[\\.]', '\\')
# [1] TRUE

# In PCRE, this wouldn't have been a mistake.
grepl('[\\.]', c('\\', '.'), perl = TRUE)
# [1] FALSE TRUE



Thanks, I didn't realize that escaping in PCRE was optional.
Escaping is optional only in brackets []. Without them it becomes 
mandatory if we want to catch just "." not any character :


grepl('.', c('\\', '.'), perl = TRUE)
#[1] TRUE TRUE

Best,
Serguei.




So the default exportPattern line could be

   exportPattern("^[^.]")

and it would work even if things were changed so that PCRE was used 
instead of TRE.


Duncan Murdoch

__
R-package-devel@r-project.org mailing list
https://stat.ethz.ch/mailman/listinfo/r-package-devel


__
R-package-devel@r-project.org mailing list
https://stat.ethz.ch/mailman/listinfo/r-package-devel


Re: [R-pkg-devel] [External] [External] RcmdrPlugin.HH_1.1-48.tar.gz

2024-03-07 Thread Duncan Murdoch

On 07/03/2024 4:16 a.m., Ivan Krylov wrote:

On Wed, 6 Mar 2024 13:46:55 -0500
Duncan Murdoch  wrote:


is this just a more or less harmless error, thinking that
the dot needs escaping


I think it's this one. You are absolutely right that the dot doesn't
need escaping in either TRE (which is what's used inside exportPattern)
or PCRE. In PRCE, this regular expression would have worked as intended:

# We do match backslashes by mistake.
grepl('[\\.]', '\\')
# [1] TRUE

# In PCRE, this wouldn't have been a mistake.
grepl('[\\.]', c('\\', '.'), perl = TRUE)
# [1] FALSE TRUE



Thanks, I didn't realize that escaping in PCRE was optional.

So the default exportPattern line could be

  exportPattern("^[^.]")

and it would work even if things were changed so that PCRE was used 
instead of TRE.


Duncan Murdoch

__
R-package-devel@r-project.org mailing list
https://stat.ethz.ch/mailman/listinfo/r-package-devel


Re: [R-pkg-devel] [External] [External] RcmdrPlugin.HH_1.1-48.tar.gz

2024-03-07 Thread Ivan Krylov via R-package-devel
On Wed, 6 Mar 2024 13:46:55 -0500
Duncan Murdoch  wrote:

> is this just a more or less harmless error, thinking that 
> the dot needs escaping

I think it's this one. You are absolutely right that the dot doesn't
need escaping in either TRE (which is what's used inside exportPattern)
or PCRE. In PRCE, this regular expression would have worked as intended:

# We do match backslashes by mistake.
grepl('[\\.]', '\\')
# [1] TRUE

# In PCRE, this wouldn't have been a mistake.
grepl('[\\.]', c('\\', '.'), perl = TRUE)
# [1] FALSE TRUE

-- 
Best regards,
Ivan

__
R-package-devel@r-project.org mailing list
https://stat.ethz.ch/mailman/listinfo/r-package-devel


Re: [R-pkg-devel] [External] [External] RcmdrPlugin.HH_1.1-48.tar.gz

2024-03-06 Thread Duncan Murdoch

On 06/03/2024 1:00 p.m., Martin Maechler wrote:

Richard M Heiberger
 on Wed, 6 Mar 2024 17:10:50 + writes:


 > Thank you, I will do that reversion in a few days.

(good; I'm sorry I did not see this, before I replied to Joshua's)

 > Before I do, I want to ask if the default export generated by R CMD 
build should be changed.
 > the default is  exportPattern("."), which seems to be the cause of the 
problem.
 > Might the default be changed to exportPattern("^[^\\.]") ?

That's a good suggestion in my view.


One thing I don't understand about that suggestion (which is taken from 
WRE, I'm not blaming Rich for it):  why include the backslash in the 
negated character class? Does R ever create variables starting with a 
backslash, or is this just a more or less harmless error, thinking that 
the dot needs escaping?


Duncan Murdoch


That default *was* sensible when namespaces were
introduced (~ 2004?): It did indeed ensure that the package worked
entirely as before there were namespaces -- always everything
was "exported", i.e. publicly visible and part of the implicit
package API.

And such 100%-backcompatibility was of course sensible to ease
transition. But we are ca. 20 years later now, and I guess that
most active R users (incl package developers) either don't know
or then hardly remember that R had no namespaces originally.

I see it only in the tools pkg hidden  writeDefaultNamespace()
which is used only once in tools:::.build_packages()

## add NAMESPACE if the author didn't write one
if(!file.exists(namespace <- file.path(pkgname, "NAMESPACE")) ) {
messageLog(Log, "creating default NAMESPACE file")
writeDefaultNamespace(namespace)
}

Note that the "Bible" on R packages has always been
'Writing R Extensions' - in the R sources,  doc/manual/R-exts.texi

It has -- *since* svn rev 23392,  003-02-27 19:02:45 +0100
   by Luke Tierney and commit message
  "Added name space support for packages that do not use methods."

the text, e.g., at
   
https://cran.r-project.org/doc/manuals/R-exts.html#Specifying-imports-and-exports


For packages with many variables to export it may be more convenient
to specify the names to export with a regular expression using
‘exportPattern’.  The directive



  exportPattern("^[^\\.]")



exports all variables that do not start with a period.  However, such
broad patterns are not recommended for production code: it is better to
list all exports or use narrowly-defined groups.  .


so I agree we should change the default.
The R code above shows that the user does get a message about
automatic NAMESPACE creation.

If there are cases, where people need to export even .,
they should have to consciously choose so.

Martin




 >> On Mar 6, 2024, at 11:57, Joshua Ulrich  wrote:
 >>
 >> On Wed, Mar 6, 2024 at 1:03 AM Richard M. Heiberger  
wrote:
 >>>
 >>> Thank you Duncan, Jeff, Ivan.
 >>>
 >>> I did all that Duncan and Jeff suggested, plus a bit more that 
appeared to be necessary.
 >>> All of what I did is documented in the RcmdrPlugin.HH/NEWS file.
 >>>
 >>> Ivan's comments were received after I sent 1.1-50 to CRAN and it was 
accepted.
 >>>
 >> I recommend you revert all the changes you made that are documented in
 >> the package NEWS, and at minimum follow Ivan's advice to use
 >> exportPattern("^[^\\.]") instead of exportPattern("."). It would be
 >> even better to follow the advice in Writing R Extensions and list each
 >> exported object individually.
 >>
 >>> I suggest that my notes in the NEWS file, perhaps augmented with 
Ivan's comments,
 >>> might be added to utils/man/globalVariables.Rd and to the
 >>> "
 >>> section ‘Package
 >>> structure’ in the ‘Writing R Extensions’ manual.
 >>> "
 >>>
 >> That section of Writing R Extensions specifically says not to do what 
you did.
 >>
 >> Beware of patterns which include names starting with a period: some
 >> of these are internal-only variables and should never be exported,
 >> e.g. ‘.__S3MethodsTable__.’ (and loading excludes known cases).
 >>
 >> Duncan pointed out that '.__global__' is an internal-only variable
 >> created by globalVariables(), which means it should never be exported
 >> by a package. Imagine the number of conflicts there would be if every
 >> package that used globalVariables() exported the '.__global__'
 >> object... there would probably be thousands, yikes!
 >>
 >> It's possible that future versions of 'R CMD check' will error if
 >> there are any incorrectly exported internal variables (like
 >> '.__global__'), which would cause your package to fail.
 >>
 >> Best,
 >> Josh
 >>
 >>
 >>>
  On Mar 6, 2024, at 01:38, Ivan Krylov  wrote:
 
  В Tue, 5 Mar 2024 22:41:32 +
  "Richard M. 

Re: [R-pkg-devel] [External] [External] RcmdrPlugin.HH_1.1-48.tar.gz

2024-03-06 Thread Martin Maechler
> Richard M Heiberger 
> on Wed, 6 Mar 2024 17:10:50 + writes:

> Thank you, I will do that reversion in a few days.

(good; I'm sorry I did not see this, before I replied to Joshua's)

> Before I do, I want to ask if the default export generated by R CMD build 
should be changed.
> the default is  exportPattern("."), which seems to be the cause of the 
problem.
> Might the default be changed to exportPattern("^[^\\.]") ?

That's a good suggestion in my view.
That default *was* sensible when namespaces were
introduced (~ 2004?): It did indeed ensure that the package worked
entirely as before there were namespaces -- always everything
was "exported", i.e. publicly visible and part of the implicit
package API.

And such 100%-backcompatibility was of course sensible to ease
transition. But we are ca. 20 years later now, and I guess that
most active R users (incl package developers) either don't know
or then hardly remember that R had no namespaces originally.

I see it only in the tools pkg hidden  writeDefaultNamespace()
which is used only once in tools:::.build_packages()

## add NAMESPACE if the author didn't write one
if(!file.exists(namespace <- file.path(pkgname, "NAMESPACE")) ) {
messageLog(Log, "creating default NAMESPACE file")
writeDefaultNamespace(namespace)
}

Note that the "Bible" on R packages has always been
'Writing R Extensions' - in the R sources,  doc/manual/R-exts.texi

It has -- *since* svn rev 23392,  003-02-27 19:02:45 +0100 
  by Luke Tierney and commit message
  "Added name space support for packages that do not use methods."

the text, e.g., at
  
https://cran.r-project.org/doc/manuals/R-exts.html#Specifying-imports-and-exports

>For packages with many variables to export it may be more convenient
> to specify the names to export with a regular expression using
> ‘exportPattern’.  The directive

>  exportPattern("^[^\\.]")

> exports all variables that do not start with a period.  However, such
> broad patterns are not recommended for production code: it is better to
> list all exports or use narrowly-defined groups.  .

so I agree we should change the default.
The R code above shows that the user does get a message about
automatic NAMESPACE creation.

If there are cases, where people need to export even .,
they should have to consciously choose so.

Martin




>> On Mar 6, 2024, at 11:57, Joshua Ulrich  wrote:
>> 
>> On Wed, Mar 6, 2024 at 1:03 AM Richard M. Heiberger  
wrote:
>>> 
>>> Thank you Duncan, Jeff, Ivan.
>>> 
>>> I did all that Duncan and Jeff suggested, plus a bit more that appeared 
to be necessary.
>>> All of what I did is documented in the RcmdrPlugin.HH/NEWS file.
>>> 
>>> Ivan's comments were received after I sent 1.1-50 to CRAN and it was 
accepted.
>>> 
>> I recommend you revert all the changes you made that are documented in
>> the package NEWS, and at minimum follow Ivan's advice to use
>> exportPattern("^[^\\.]") instead of exportPattern("."). It would be
>> even better to follow the advice in Writing R Extensions and list each
>> exported object individually.
>> 
>>> I suggest that my notes in the NEWS file, perhaps augmented with Ivan's 
comments,
>>> might be added to utils/man/globalVariables.Rd and to the
>>> "
>>> section ‘Package
>>> structure’ in the ‘Writing R Extensions’ manual.
>>> "
>>> 
>> That section of Writing R Extensions specifically says not to do what 
you did.
>> 
>> Beware of patterns which include names starting with a period: some
>> of these are internal-only variables and should never be exported,
>> e.g. ‘.__S3MethodsTable__.’ (and loading excludes known cases).
>> 
>> Duncan pointed out that '.__global__' is an internal-only variable
>> created by globalVariables(), which means it should never be exported
>> by a package. Imagine the number of conflicts there would be if every
>> package that used globalVariables() exported the '.__global__'
>> object... there would probably be thousands, yikes!
>> 
>> It's possible that future versions of 'R CMD check' will error if
>> there are any incorrectly exported internal variables (like
>> '.__global__'), which would cause your package to fail.
>> 
>> Best,
>> Josh
>> 
>> 
>>> 
 On Mar 6, 2024, at 01:38, Ivan Krylov  wrote:
 
 В Tue, 5 Mar 2024 22:41:32 +
 "Richard M. Heiberger"  пишет:
 
> Undocumented code objects:
> '.__global__'
> All user-level objects in a package should have documentation
> entries. See chapter 'Writing R documentation files' in the 'Writing R
> Extensions' manual.
 
 This object is not here for the user of the package. If you don't
 export it, there will be no WARNING about it 

Re: [R-pkg-devel] [External] [External] RcmdrPlugin.HH_1.1-48.tar.gz

2024-03-06 Thread Richard M. Heiberger
Thank you, I will do that reversion in a few days.

Before I do, I want to ask if the default export generated by R CMD build 
should be changed.
the default is  exportPattern("."), which seems to be the cause of the problem.
Might the default be changed to exportPattern("^[^\\.]") ?

> On Mar 6, 2024, at 11:57, Joshua Ulrich  wrote:
>
> On Wed, Mar 6, 2024 at 1:03 AM Richard M. Heiberger  wrote:
>>
>> Thank you Duncan, Jeff, Ivan.
>>
>> I did all that Duncan and Jeff suggested, plus a bit more that appeared to 
>> be necessary.
>> All of what I did is documented in the RcmdrPlugin.HH/NEWS file.
>>
>> Ivan's comments were received after I sent 1.1-50 to CRAN and it was 
>> accepted.
>>
> I recommend you revert all the changes you made that are documented in
> the package NEWS, and at minimum follow Ivan's advice to use
> exportPattern("^[^\\.]") instead of exportPattern("."). It would be
> even better to follow the advice in Writing R Extensions and list each
> exported object individually.
>
>> I suggest that my notes in the NEWS file, perhaps augmented with Ivan's 
>> comments,
>> might be added to utils/man/globalVariables.Rd and to the
>> "
>> section ‘Package
>> structure’ in the ‘Writing R Extensions’ manual.
>> "
>>
> That section of Writing R Extensions specifically says not to do what you did.
>
>Beware of patterns which include names starting with a period: some
>of these are internal-only variables and should never be exported,
>e.g. ‘.__S3MethodsTable__.’ (and loading excludes known cases).
>
> Duncan pointed out that '.__global__' is an internal-only variable
> created by globalVariables(), which means it should never be exported
> by a package. Imagine the number of conflicts there would be if every
> package that used globalVariables() exported the '.__global__'
> object... there would probably be thousands, yikes!
>
> It's possible that future versions of 'R CMD check' will error if
> there are any incorrectly exported internal variables (like
> '.__global__'), which would cause your package to fail.
>
> Best,
> Josh
>
>
>>
>>> On Mar 6, 2024, at 01:38, Ivan Krylov  wrote:
>>>
>>> В Tue, 5 Mar 2024 22:41:32 +
>>> "Richard M. Heiberger"  пишет:
>>>
 Undocumented code objects:
  '.__global__'
 All user-level objects in a package should have documentation
 entries. See chapter 'Writing R documentation files' in the 'Writing R
 Extensions' manual.
>>>
>>> This object is not here for the user of the package. If you don't
>>> export it, there will be no WARNING about it being undocumented. This
>>> variable is exported because of exportPattern(".") in the file
>>> NAMESPACE. The lone dot is a regular expression that matches any name
>>> of an R object.
>>>
>>> If you don't want to manually list your exports in the NAMESPACE file
>>> (which can get tedious) or generate it (which takes additional
>>> dependencies and build steps), you can use exportPattern('^[^\\.]') to
>>> export everything except objects with a name starting with a period:
>>> https://cran.r-project.org/doc/manuals/R-exts.html#Specifying-imports-and-exports
>>>
>>> --
>>> Best regards,
>>> Ivan
>>
>> __
>> R-package-devel@r-project.org mailing list
>> https://stat.ethz.ch/mailman/listinfo/r-package-devel
>
>
>
> --
> Joshua Ulrich  |  about.me/joshuaulrich
> FOSS Trading  |  http://www.fosstrading.com/


__
R-package-devel@r-project.org mailing list
https://stat.ethz.ch/mailman/listinfo/r-package-devel