R devel - Jun 2019 - Suggested Patch: Library returns matching installed packages when typo present

Hi Luke,

Thank you for your response.

On 6/21/19 10:56 AM, Tierney, Luke wrote:

Thanks for the suggestion. However I don't think it is the right way
to go. I also don't care for what install.packages() does. Signaling a
warning and then an error means someone has to catch both the error
and the warning, or suppress the warning, in order to handle the error
programmatically.

I do care for what install.packages() does because it's preferable
to have consistency in the user interface.

I see that the proposed patch does return both an error and a warning
but as far as I understand it, library()'s main/intended use is interactive
and
there are other functions available for programmatic use cases.



Now that library() signals a structured error there are other options.
One possibility, at least as an interim, is to define a
conditionMessage method, e.g. as

conditionMessage.packageNotFoundError <- function(c) {
     lib.loc <- c$lib.loc
     msg <- c$message
     package <- c$package
     if(length(lib.loc)) {
         allpkgs <- .packages(TRUE, lib.loc)
         if (!is.na(w <- match(tolower(package), tolower(allpkgs)))) {
             msg2 <- sprintf("Perhaps you meant %s ?",
sQuote(allpkgs[w]))
             return(paste(msg, msg2, sep = "\n"))
         }
     }
     msg
}

This is something you can do yourself, though it is generally not a
good idea to define a method when you don't own either the generic or
the class.

Something that would be useful and is being considered is having a
mechanism for registering default condition handlers. This would allow
the condition to be re-signaled with a custom class and then having
a custom conditionMessage method is less likely to cause conflicts.

I'd argue that this is quite useful especially for new users and that
creating
condition handlers may involve more than what is needed for interactive use.


Best,

Marcel



Also worth looking into is establishing a restart around the error
signal.  This would allow an IDE, for example, to provide a dialog for
choosing the alternate package and retrying without the need to call
library() again. This is currently done in loadNamespace() but not yet
in library(). Can have downsides as well -- if the library() call is
in a notebook, for example, then you do want to fix the call ...  It
is arguably more useful in loadNamespace since that can get called
implicitly inside a longer computation that you don't necessarily want
to start over.

Best,

luke

On Fri, 21 Jun 2019, Marcel Ramos wrote:



Dear R-core devs,

I hope this email finds you well.

Please see the proposed patch to R-devel below:

Scenario:

When loading a package using `library`, a package may not be found if the cases
are not matching:

```


library(ORG.Hs.eg.db)


Error in library(ORG.Hs.eg.db) :
 there is no package called 'ORG.Hs.eg.db'
```


Suggested Patch:

Returns a message matching what `install.packages` returns in such situations:

```


library("ORG.Hs.eg.db")


Error in library("ORG.Hs.eg.db") :
  there is no package called 'ORG.Hs.eg.db'
In addition: Warning message:
Perhaps you meant 'org.Hs.eg.db' ?
```

This patch will be helpful with 'fat-finger' typos. It will match a
package
called with `library` against installed packages.


svn diff:

Index: src/library/base/R/library.R
==================================================================---
src/library/base/R/library.R    (revision 76727)
+++ src/library/base/R/library.R    (working copy)
@@ -300,8 +300,13 @@
        pkgpath <- find.package(package, lib.loc, quiet = TRUE,
                                    verbose = verbose)
            if(length(pkgpath) == 0L) {
-                if(length(lib.loc) && !logical.return)
+                if(length(lib.loc) && !logical.return) {
+                    allpkgs <- .packages(TRUE, lib.loc)
+                    if (!is.na(w <- match(tolower(package),
tolower(allpkgs))))
+                        warning(sprintf("Perhaps you meant %s ?",
+                            sQuote(allpkgs[w])), call. = FALSE, domain = NA)
                    stop(packageNotFoundError(package, lib.loc, sys.call()))
+                }
                txt <- if(length(lib.loc))
                    gettextf("there is no package called %s",
sQuote(package))
                else


Thank you!

Best regards,

Marcel



--
Marcel Ramos
Bioconductor Core Team
Roswell Park Comprehensive Care Center
Dept. of Biostatistics & Bioinformatics
Elm & Carlton Streets
Buffalo, New York 14263


This email message may contain legally privileged and/or confidential
information.  If you are not the intended recipient(s), or the employee or agent
responsible for the delivery of this message to the intended recipient(s), you
are hereby notified that any disclosure, copying, distribution, or use of this
email message is prohibited.  If you have received this message in error, please
notify the sender immediately by e-mail and delete this email message from your
computer. Thank you.
        [[alternative HTML version deleted]]

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







This email message may contain legally privileged and/or confidential
information.  If you are not the intended recipient(s), or the employee or agent
responsible for the delivery of this message to the intended recipient(s), you
are hereby notified that any disclosure, copying, distribution, or use of this
email message is prohibited.  If you have received this message in error, please
notify the sender immediately by e-mail and delete this email message from your
computer. Thank you.
	[[alternative HTML version deleted]]

> On 6/21/19 10:56 AM, Tierney, Luke wrote:
> [...]
> Something that would be useful and is being considered is having a
> mechanism for registering default condition handlers. This would allow
> the condition to be re-signaled with a custom class and then having
> a custom conditionMessage method is less likely to cause conflicts.
Is it correct that you are proposing something that allows us to do:

registerDefaultConditionHandlers(
  packageStartupMessage = function(c) {
    ## Do something with condition 'c'
    ...
    ## Suppress futher processing
    invokeRestart("muffleMessage")
  }
)

at the core, which avoids having us to wrap up calls in
withCallingHandlers() at top-level calls, e.g.
> withCallingHandlers({
  library("foobar"),
}, packageStartupMessage = function(c) {
  ## Do something with condition 'c'
  ...
  ## Suppress futher processing
  invokeRestart("muffleMessage")
})

?

Then, if I read this correctly, I'd say, this would be a very useful
addition to base R.  This will provide a core framework that opens up
for several neat extensions, e.g. the one that Marcel suggests - some
people prefer a message/warning on misspelled package names, while
others might want to see if it can be automatically installed, and so
on.  And, it will (=should) be all in the hands of the end user to
control this, i.e. various packages should not override this similar
to how we don't expect them to override other personal R settings we
have.

With this in place, it's not hard to imagine a third-party package
that provides useful handlers that users can pick from, e.g.

  buttlr::i_am_a("first_time_r_user")

to get extra information for some of the common warnings and errors,
which is in the same spirit as your idea on:
> [...] This would allow an IDE, for example, to provide a dialog for
> choosing the alternate package and retrying without the need to call
> library() again. [...]
I also think such a framework could replace some of the "legacy
handlers" we currently have in place, e.g. R options 'warn',
'warnPartialMatchArgs', '...', and even 'error', and
give more
granular control over those use cases.  For instance, instead of a
warning or a partial argument match, I might want to produce an error
unless it comes from one particular package, say, which is something
that is a bit tricky to do today.

/Henrik

PS. Somewhat related to this, standardizing muffling and signaling of
conditions could be worth looking into as well.  For instance, being
able to resignal an error 'e' with a *generic* signalCondition(e)
instead of having to know that you should call stop(e) for 'error'
conditions and maybe another function if the error is of another
class.

On Fri, Jun 21, 2019 at 8:55 AM Marcel Ramos
<Marcel.Ramos at roswellpark.org> wrote:
>
> Hi Luke,
>
> Thank you for your response.
>
> On 6/21/19 10:56 AM, Tierney, Luke wrote:
>
> Thanks for the suggestion. However I don't think it is the right way
> to go. I also don't care for what install.packages() does. Signaling a
> warning and then an error means someone has to catch both the error
> and the warning, or suppress the warning, in order to handle the error
> programmatically.
>
> I do care for what install.packages() does because it's preferable
> to have consistency in the user interface.
>
> I see that the proposed patch does return both an error and a warning
> but as far as I understand it, library()'s main/intended use is
interactive and
> there are other functions available for programmatic use cases.
>
>
>
> Now that library() signals a structured error there are other options.
> One possibility, at least as an interim, is to define a
> conditionMessage method, e.g. as
>
> conditionMessage.packageNotFoundError <- function(c) {
>      lib.loc <- c$lib.loc
>      msg <- c$message
>      package <- c$package
>      if(length(lib.loc)) {
>          allpkgs <- .packages(TRUE, lib.loc)
>          if (!is.na(w <- match(tolower(package), tolower(allpkgs)))) {
>              msg2 <- sprintf("Perhaps you meant %s ?",
sQuote(allpkgs[w]))
>              return(paste(msg, msg2, sep = "\n"))
>          }
>      }
>      msg
> }
>
> This is something you can do yourself, though it is generally not a
> good idea to define a method when you don't own either the generic or
> the class.
>
> Something that would be useful and is being considered is having a
> mechanism for registering default condition handlers. This would allow
> the condition to be re-signaled with a custom class and then having
> a custom conditionMessage method is less likely to cause conflicts.
>
> I'd argue that this is quite useful especially for new users and that
creating
> condition handlers may involve more than what is needed for interactive
use.
>
>
> Best,
>
> Marcel
>
>
>
> Also worth looking into is establishing a restart around the error
> signal.  This would allow an IDE, for example, to provide a dialog for
> choosing the alternate package and retrying without the need to call
> library() again. This is currently done in loadNamespace() but not yet
> in library(). Can have downsides as well -- if the library() call is
> in a notebook, for example, then you do want to fix the call ...  It
> is arguably more useful in loadNamespace since that can get called
> implicitly inside a longer computation that you don't necessarily want
> to start over.
>
> Best,
>
> luke
>
> On Fri, 21 Jun 2019, Marcel Ramos wrote:
>
>
>
> Dear R-core devs,
>
> I hope this email finds you well.
>
> Please see the proposed patch to R-devel below:
>
> Scenario:
>
> When loading a package using `library`, a package may not be found if the
cases are not matching:
>
> ```
>
>
> library(ORG.Hs.eg.db)
>
>
> Error in library(ORG.Hs.eg.db) :
>  there is no package called 'ORG.Hs.eg.db'
> ```
>
>
> Suggested Patch:
>
> Returns a message matching what `install.packages` returns in such
situations:
>
> ```
>
>
> library("ORG.Hs.eg.db")
>
>
> Error in library("ORG.Hs.eg.db") :
>   there is no package called 'ORG.Hs.eg.db'
> In addition: Warning message:
> Perhaps you meant 'org.Hs.eg.db' ?
> ```
>
> This patch will be helpful with 'fat-finger' typos. It will match a
package
> called with `library` against installed packages.
>
>
> svn diff:
>
> Index: src/library/base/R/library.R
> ==================================================================> ---
src/library/base/R/library.R    (revision 76727)
> +++ src/library/base/R/library.R    (working copy)
> @@ -300,8 +300,13 @@
>         pkgpath <- find.package(package, lib.loc, quiet = TRUE,
>                                     verbose = verbose)
>             if(length(pkgpath) == 0L) {
> -                if(length(lib.loc) && !logical.return)
> +                if(length(lib.loc) && !logical.return) {
> +                    allpkgs <- .packages(TRUE, lib.loc)
> +                    if (!is.na(w <- match(tolower(package),
tolower(allpkgs))))
> +                        warning(sprintf("Perhaps you meant %s
?",
> +                            sQuote(allpkgs[w])), call. = FALSE, domain =
NA)
>                     stop(packageNotFoundError(package, lib.loc,
sys.call()))
> +                }
>                 txt <- if(length(lib.loc))
>                     gettextf("there is no package called %s",
sQuote(package))
>                 else
>
>
> Thank you!
>
> Best regards,
>
> Marcel
>
>
>
> --
> Marcel Ramos
> Bioconductor Core Team
> Roswell Park Comprehensive Care Center
> Dept. of Biostatistics & Bioinformatics
> Elm & Carlton Streets
> Buffalo, New York 14263
>
>
> This email message may contain legally privileged and/or confidential
information.  If you are not the intended recipient(s), or the employee or agent
responsible for the delivery of this message to the intended recipient(s), you
are hereby notified that any disclosure, copying, distribution, or use of this
email message is prohibited.  If you have received this message in error, please
notify the sender immediately by e-mail and delete this email message from your
computer. Thank you.
>         [[alternative HTML version deleted]]
>
> ______________________________________________
> R-devel at r-project.org<mailto:R-devel at r-project.org> mailing
list
> https://stat.ethz.ch/mailman/listinfo/r-devel
>
>
>
>
>
>
>
> This email message may contain legally privileged and/or confidential
information.  If you are not the intended recipient(s), or the employee or agent
responsible for the delivery of this message to the intended recipient(s), you
are hereby notified that any disclosure, copying, distribution, or use of this
email message is prohibited.  If you have received this message in error, please
notify the sender immediately by e-mail and delete this email message from your
computer. Thank you.
>         [[alternative HTML version deleted]]
>
> ______________________________________________
> R-devel at r-project.org mailing list
> https://stat.ethz.ch/mailman/listinfo/r-devel

On Fri, 21 Jun 2019, Henrik Bengtsson wrote:
>> On 6/21/19 10:56 AM, Tierney, Luke wrote:
>> [...]
>> Something that would be useful and is being considered is having a
>> mechanism for registering default condition handlers. This would allow
>> the condition to be re-signaled with a custom class and then having
>> a custom conditionMessage method is less likely to cause conflicts.
>
> Is it correct that you are proposing something that allows us to do:
>
> registerDefaultConditionHandlers(
>  packageStartupMessage = function(c) {
>    ## Do something with condition 'c'
>    ...
>    ## Suppress futher processing
>    invokeRestart("muffleMessage")
>  }
> )
>
> at the core, which avoids having us to wrap up calls in
> withCallingHandlers() at top-level calls, e.g.
>
>> withCallingHandlers({
>  library("foobar"),
> }, packageStartupMessage = function(c) {
>  ## Do something with condition 'c'
>  ...
>  ## Suppress futher processing
>  invokeRestart("muffleMessage")
> })
>
> ?
>
> Then, if I read this correctly, I'd say, this would be a very useful
> addition to base R.  This will provide a core framework that opens up
> for several neat extensions, e.g. the one that Marcel suggests - some
> people prefer a message/warning on misspelled package names, while
> others might want to see if it can be automatically installed, and so
> on.  And, it will (=should) be all in the hands of the end user to
> control this, i.e. various packages should not override this similar
> to how we don't expect them to override other personal R settings we
> have.
Something along those lines. Not thoroughly thought through yet, and
there are lots of other things ahead in the queue ...
>
> With this in place, it's not hard to imagine a third-party package
> that provides useful handlers that users can pick from, e.g.
>
>  buttlr::i_am_a("first_time_r_user")
>
> to get extra information for some of the common warnings and errors,
> which is in the same spirit as your idea on:
>
>> [...] This would allow an IDE, for example, to provide a dialog for
>> choosing the alternate package and retrying without the need to call
>> library() again. [...]
>
> I also think such a framework could replace some of the "legacy
> handlers" we currently have in place, e.g. R options 'warn',
> 'warnPartialMatchArgs', '...', and even 'error',
and give more
> granular control over those use cases.  For instance, instead of a
> warning or a partial argument match, I might want to produce an error
> unless it comes from one particular package, say, which is something
> that is a bit tricky to do today.
As I am sure you know changing things that have been around for a long
time is a lot more complicated than adding new things ...
>
> /Henrik
>
> PS. Somewhat related to this, standardizing muffling and signaling of
> conditions could be worth looking into as well.  For instance, being
> able to resignal an error 'e' with a *generic* signalCondition(e)
> instead of having to know that you should call stop(e) for 'error'
> conditions and maybe another function if the error is of another
> class.
No. This is working as intended. Signaling protocols and class
hierarchies are separate. _Usually_ you will signal an error with
stop() and a warning with warning() but you can do it the other way as
well. Condition classes determine what handlers are eligible. The
choice of signaling function determines the protocol for signaling
whatever condition the function is given, including what extra
restarts might be available and what happens if no handler is
available or all handlers decline by returning. stop() and
warning/message both use signalCondition to allow handlers to handle
the condition.  For stop(), the protocol guarantees that stop will
never return since it invokes an abort restart if the condition is not
handled. warning/message signal with a muffling restart in place. If
you use signalCondition directly you can establish your own protocol.

Best,

luke

> On Fri, Jun 21, 2019 at 8:55 AM Marcel Ramos
> <Marcel.Ramos at roswellpark.org> wrote:
>>
>> Hi Luke,
>>
>> Thank you for your response.
>>
>> On 6/21/19 10:56 AM, Tierney, Luke wrote:
>>
>> Thanks for the suggestion. However I don't think it is the right
way
>> to go. I also don't care for what install.packages() does.
Signaling a
>> warning and then an error means someone has to catch both the error
>> and the warning, or suppress the warning, in order to handle the error
>> programmatically.
>>
>> I do care for what install.packages() does because it's preferable
>> to have consistency in the user interface.
>>
>> I see that the proposed patch does return both an error and a warning
>> but as far as I understand it, library()'s main/intended use is
interactive and
>> there are other functions available for programmatic use cases.
>>
>>
>>
>> Now that library() signals a structured error there are other options.
>> One possibility, at least as an interim, is to define a
>> conditionMessage method, e.g. as
>>
>> conditionMessage.packageNotFoundError <- function(c) {
>>      lib.loc <- c$lib.loc
>>      msg <- c$message
>>      package <- c$package
>>      if(length(lib.loc)) {
>>          allpkgs <- .packages(TRUE, lib.loc)
>>          if (!is.na(w <- match(tolower(package), tolower(allpkgs))))
{
>>              msg2 <- sprintf("Perhaps you meant %s ?",
sQuote(allpkgs[w]))
>>              return(paste(msg, msg2, sep = "\n"))
>>          }
>>      }
>>      msg
>> }
>>
>> This is something you can do yourself, though it is generally not a
>> good idea to define a method when you don't own either the generic
or
>> the class.
>>
>> Something that would be useful and is being considered is having a
>> mechanism for registering default condition handlers. This would allow
>> the condition to be re-signaled with a custom class and then having
>> a custom conditionMessage method is less likely to cause conflicts.
>>
>> I'd argue that this is quite useful especially for new users and
that creating
>> condition handlers may involve more than what is needed for interactive
use.
>>
>>
>> Best,
>>
>> Marcel
>>
>>
>>
>> Also worth looking into is establishing a restart around the error
>> signal.  This would allow an IDE, for example, to provide a dialog for
>> choosing the alternate package and retrying without the need to call
>> library() again. This is currently done in loadNamespace() but not yet
>> in library(). Can have downsides as well -- if the library() call is
>> in a notebook, for example, then you do want to fix the call ...  It
>> is arguably more useful in loadNamespace since that can get called
>> implicitly inside a longer computation that you don't necessarily
want
>> to start over.
>>
>> Best,
>>
>> luke
>>
>> On Fri, 21 Jun 2019, Marcel Ramos wrote:
>>
>>
>>
>> Dear R-core devs,
>>
>> I hope this email finds you well.
>>
>> Please see the proposed patch to R-devel below:
>>
>> Scenario:
>>
>> When loading a package using `library`, a package may not be found if
the cases are not matching:
>>
>> ```
>>
>>
>> library(ORG.Hs.eg.db)
>>
>>
>> Error in library(ORG.Hs.eg.db) :
>>  there is no package called 'ORG.Hs.eg.db'
>> ```
>>
>>
>> Suggested Patch:
>>
>> Returns a message matching what `install.packages` returns in such
situations:
>>
>> ```
>>
>>
>> library("ORG.Hs.eg.db")
>>
>>
>> Error in library("ORG.Hs.eg.db") :
>>   there is no package called 'ORG.Hs.eg.db'
>> In addition: Warning message:
>> Perhaps you meant 'org.Hs.eg.db' ?
>> ```
>>
>> This patch will be helpful with 'fat-finger' typos. It will
match a package
>> called with `library` against installed packages.
>>
>>
>> svn diff:
>>
>> Index: src/library/base/R/library.R
>>
==================================================================>> ---
src/library/base/R/library.R    (revision 76727)
>> +++ src/library/base/R/library.R    (working copy)
>> @@ -300,8 +300,13 @@
>>         pkgpath <- find.package(package, lib.loc, quiet = TRUE,
>>                                     verbose = verbose)
>>             if(length(pkgpath) == 0L) {
>> -                if(length(lib.loc) && !logical.return)
>> +                if(length(lib.loc) && !logical.return) {
>> +                    allpkgs <- .packages(TRUE, lib.loc)
>> +                    if (!is.na(w <- match(tolower(package),
tolower(allpkgs))))
>> +                        warning(sprintf("Perhaps you meant %s
?",
>> +                            sQuote(allpkgs[w])), call. = FALSE, domain
= NA)
>>                     stop(packageNotFoundError(package, lib.loc,
sys.call()))
>> +                }
>>                 txt <- if(length(lib.loc))
>>                     gettextf("there is no package called %s",
sQuote(package))
>>                 else
>>
>>
>> Thank you!
>>
>> Best regards,
>>
>> Marcel
>>
>>
>>
>> --
>> Marcel Ramos
>> Bioconductor Core Team
>> Roswell Park Comprehensive Care Center
>> Dept. of Biostatistics & Bioinformatics
>> Elm & Carlton Streets
>> Buffalo, New York 14263
>>
>>
>> This email message may contain legally privileged and/or confidential
information.  If you are not the intended recipient(s), or the employee or agent
responsible for the delivery of this message to the intended recipient(s), you
are hereby notified that any disclosure, copying, distribution, or use of this
email message is prohibited.  If you have received this message in error, please
notify the sender immediately by e-mail and delete this email message from your
computer. Thank you.
>>         [[alternative HTML version deleted]]
>>
>> ______________________________________________
>> R-devel at r-project.org<mailto:R-devel at r-project.org> mailing
list
>> https://stat.ethz.ch/mailman/listinfo/r-devel
>>
>>
>>
>>
>>
>>
>>
>> This email message may contain legally privileged and/or confidential
information.  If you are not the intended recipient(s), or the employee or agent
responsible for the delivery of this message to the intended recipient(s), you
are hereby notified that any disclosure, copying, distribution, or use of this
email message is prohibited.  If you have received this message in error, please
notify the sender immediately by e-mail and delete this email message from your
computer. Thank you.
>>         [[alternative HTML version deleted]]
>>
>> ______________________________________________
>> R-devel at r-project.org mailing list
>> https://stat.ethz.ch/mailman/listinfo/r-devel
>
-- 
Luke Tierney
Ralph E. Wareham Professor of Mathematical Sciences
University of Iowa                  Phone:             319-335-3386
Department of Statistics and        Fax:               319-335-3017
    Actuarial Science
241 Schaeffer Hall                  email:   luke-tierney at uiowa.edu
Iowa City, IA 52242                 WWW:  http://www.stat.uiowa.edu