Re: [Rd] [External] Re: Repeated library() of one package with different include.only= entries

2024-04-15 Thread Michael Chirico
OK, I've moved your suggestions into the tracker for further discussion:

https://bugs.r-project.org/show_bug.cgi?id=18703
https://bugs.r-project.org/show_bug.cgi?id=18704
https://bugs.r-project.org/show_bug.cgi?id=18705

I will tackle a patch for 18705 to start with as the lowest-hanging fruit.

On Mon, Apr 15, 2024 at 6:46 AM Martin Maechler 
wrote:

> I think we should try to advance and hopefully finalize this
> thread before we forget about it ..
>
> > Michael Chirico  n Thu, 11 Apr 2024 09:10:11 -0700 writes:
>
> >> I would assume that
>
> >> library(Matrix, include.only="isDiagonal")
>
> >> implies that only `isDiagonal` ends up on the search path
>
> > This could also be a reasonable behavior, but neither does that
> happen
> > today.
>
> Indeed; I think we should signal a warning at least in the case
> it does not happen --- namely when "parts of Matrix" are already
> in the search path.
>
> >> I think a far better approach to solve Michael's problem is simply
> to use
>
> >> fac2sparse <- Matrix::fac2sparse
>
> > This does not fully simulate attachment, e.g. running package hooks &
> > resolving Depends.
>
> (indeed; as library() is really about search() patch attchment)
>
> >> ?library could also mention using detach() followed by library() or
> >> attachNamespace() with a new include.only specification.
>
> > This is the "quite hard to accomplish" I alluded to, admittedly I
> hadn't
> > forced myself to write it all out -- maybe it's not as bad as all
> that.
> > After some iterations, today I think we'd want to do...
>
> > modify_attach <- function(pkg, new_names) {
> >   if (!startsWith(pkg, "package:")) pkg <- paste0("package:", pkg)
> >   old <- tryCatch(ls(pkg, all.names=TRUE), error=\(c) character())
> >   if (length(old)) detach(pkg)
> >   attachNamespace(.rmpkg(pkg), include.only=c(new_names, old))
> > }
>
> > Perhaps detach() could invisibly return the exported names to make
> this a
> > tiny bit easier (today it returns NULL):
>
> I agree that such changed detach() behavior would be "nice" here;
> but I wouldn't like to change its default behavior, notably as
> in 99.5% of its use, the names would not be used.
> I'd agree to add a new option for detach() to return the names
> (visibly in that case).
>
> > modify_attach <- function(pkg, new_names) {
> >   if (!startsWith(pkg, "package:")) pkg <- paste0("package:", pkg)
> >   old <- tryCatch(detach(pkg), error=\(c) character())
> >   attachNamespace(.rmpkg(pkg), include.only=c(new_names, old))
> > }
>
> > Regardless, I think your suggestion to just point to
> > detach()+attachNamespace() is reasonable enough, the rare users that
> care
> > about this are likely to be able to figure out the rest from there.
>
> So I think we agree here;  mentioning such a modify_attach()
> ... I'd use an example *without* tryCatch() as I think the user
> should choose their own line of action in such cases
> ...
> on the help page would then be appropriate.
>
> Martin
>
> > On Thu, Apr 11, 2024 at 7:37 AM  wrote:
>
> >> On Thu, 11 Apr 2024, Duncan Murdoch wrote:
> >>
> >> > On 11/04/2024 7:04 a.m., Martin Maechler wrote:
> >> >>> Michael Chirico
> >> >>>  on Mon, 8 Apr 2024 10:19:29 -0700 writes:
> >> >>
> >> >>  > Right now, attaching the same package with different
> >> include.only= has no effect:
> >> >>
> >> >>  > library(Matrix, include.only="fac2sparse")
> >> >>  > library(Matrix)
> >> >>  > ls("package:Matrix")
> >> >>  > # [1] "fac2sparse"
> >> >>
> >> >>  > ?library does not cover this case -- what is covered is
> the _loading_
> >> >>  > behavior of repeated calls:
> >> >>
> >> >>  >> [library and require] check and update the list of
> currently attached
> >> >>  > packages and do not reload a namespace which is already
> loaded
> >> >>
> >> >>  > But here we're looking at the _attach_ behavior of
> repeated calls.
> >> >>
> >> >>  > I am particularly interested in allowing the exports of a
> >> package to be
> >> >>  > built up gradually:
> >> >>
> >> >>  > library(Matrix, include.only="fac2sparse")
> >> >>  > library(Matrix, include.only="isDiagonal") #
> want:ls("package:Matrix") -->
> >> >>  > c("fac2sparse", "isDiagonal")
> >> >>  > ...
> >> >>
> >> >>  > It seems quite hard to accomplish this at the moment. Is
> the behavior to
> >> >>  > ignore new inclusions intentional? Could there be an
> argument to get
> >> >>  > different behavior?
> >> >>
> >> >> As you did not get an answer yet, ..., some remarks by an
> >> >> R-corer who has tweaked library() behavior in the past :
> >> >>
> >> >> - The `include.only = *` argument to library() has been a
> >> >>

Re: [Rd] [External] Re: Repeated library() of one package with different include.only= entries

2024-04-15 Thread Martin Maechler
I think we should try to advance and hopefully finalize this
thread before we forget about it ..

> Michael Chirico  n Thu, 11 Apr 2024 09:10:11 -0700 writes:

>> I would assume that

>> library(Matrix, include.only="isDiagonal")

>> implies that only `isDiagonal` ends up on the search path

> This could also be a reasonable behavior, but neither does that happen
> today.

Indeed; I think we should signal a warning at least in the case
it does not happen --- namely when "parts of Matrix" are already
in the search path.

>> I think a far better approach to solve Michael's problem is simply to use

>> fac2sparse <- Matrix::fac2sparse

> This does not fully simulate attachment, e.g. running package hooks &
> resolving Depends.

(indeed; as library() is really about search() patch attchment)

>> ?library could also mention using detach() followed by library() or
>> attachNamespace() with a new include.only specification.

> This is the "quite hard to accomplish" I alluded to, admittedly I hadn't
> forced myself to write it all out -- maybe it's not as bad as all that.
> After some iterations, today I think we'd want to do...

> modify_attach <- function(pkg, new_names) {
>   if (!startsWith(pkg, "package:")) pkg <- paste0("package:", pkg)
>   old <- tryCatch(ls(pkg, all.names=TRUE), error=\(c) character())
>   if (length(old)) detach(pkg)
>   attachNamespace(.rmpkg(pkg), include.only=c(new_names, old))
> }

> Perhaps detach() could invisibly return the exported names to make this a
> tiny bit easier (today it returns NULL):

I agree that such changed detach() behavior would be "nice" here;
but I wouldn't like to change its default behavior, notably as
in 99.5% of its use, the names would not be used.
I'd agree to add a new option for detach() to return the names
(visibly in that case).

> modify_attach <- function(pkg, new_names) {
>   if (!startsWith(pkg, "package:")) pkg <- paste0("package:", pkg)
>   old <- tryCatch(detach(pkg), error=\(c) character())
>   attachNamespace(.rmpkg(pkg), include.only=c(new_names, old))
> }

> Regardless, I think your suggestion to just point to
> detach()+attachNamespace() is reasonable enough, the rare users that care
> about this are likely to be able to figure out the rest from there.

So I think we agree here;  mentioning such a modify_attach()
... I'd use an example *without* tryCatch() as I think the user
should choose their own line of action in such cases
...
on the help page would then be appropriate. 

Martin

> On Thu, Apr 11, 2024 at 7:37 AM  wrote:

>> On Thu, 11 Apr 2024, Duncan Murdoch wrote:
>> 
>> > On 11/04/2024 7:04 a.m., Martin Maechler wrote:
>> >>> Michael Chirico
>> >>>  on Mon, 8 Apr 2024 10:19:29 -0700 writes:
>> >>
>> >>  > Right now, attaching the same package with different
>> include.only= has no effect:
>> >>
>> >>  > library(Matrix, include.only="fac2sparse")
>> >>  > library(Matrix)
>> >>  > ls("package:Matrix")
>> >>  > # [1] "fac2sparse"
>> >>
>> >>  > ?library does not cover this case -- what is covered is the 
_loading_
>> >>  > behavior of repeated calls:
>> >>
>> >>  >> [library and require] check and update the list of currently 
attached
>> >>  > packages and do not reload a namespace which is already loaded
>> >>
>> >>  > But here we're looking at the _attach_ behavior of repeated 
calls.
>> >>
>> >>  > I am particularly interested in allowing the exports of a
>> package to be
>> >>  > built up gradually:
>> >>
>> >>  > library(Matrix, include.only="fac2sparse")
>> >>  > library(Matrix, include.only="isDiagonal") # 
want:ls("package:Matrix") -->
>> >>  > c("fac2sparse", "isDiagonal")
>> >>  > ...
>> >>
>> >>  > It seems quite hard to accomplish this at the moment. Is the 
behavior to
>> >>  > ignore new inclusions intentional? Could there be an argument 
to get
>> >>  > different behavior?
>> >>
>> >> As you did not get an answer yet, ..., some remarks by an
>> >> R-corer who has tweaked library() behavior in the past :
>> >>
>> >> - The `include.only = *` argument to library() has been a
>> >>*relatively* recent addition {given the 25+ years of R history}:
>> >>
>> >>It was part of the extensive new features by Luke Tierney for
>> >>R 3.6.0  [r76248 | luke | 2019-03-18 17:29:35 +0100], with NEWS 
entry
>> >>
>> >>  • library() and require() now allow more control over handling
>> >>search path conflicts when packages are attached. The policy is
>> >>controlled by the new conflicts.policy option.
>> >>
>> >> - I haven't seen these (then) new features been used much, 
unfortunately,
>> >>

Re: [Rd] [External] Re: Repeated library() of one package with different include.only= entries

2024-04-11 Thread Michael Chirico
> I would assume that
>   library(Matrix, include.only="isDiagonal")
> implies that only `isDiagonal` ends up on the search path

This could also be a reasonable behavior, but neither does that happen
today.
> I think a far better approach to solve Michael's problem is simply to use
>   fac2sparse <- Matrix::fac2sparse

This does not fully simulate attachment, e.g. running package hooks &
resolving Depends.

> ?library could also mention using detach() followed by library() or
> attachNamespace() with a new include.only specification.

This is the "quite hard to accomplish" I alluded to, admittedly I hadn't
forced myself to write it all out -- maybe it's not as bad as all that.
After some iterations, today I think we'd want to do...

modify_attach = function(pkg, new_names) {
  if (!startsWith(pkg, "package:")) pkg <- paste0("package:", pkg)
  old <- tryCatch(ls(pkg, all.names=TRUE), error=\(c) character())
  if (length(old)) detach(pkg)
  attachNamespace(.rmpkg(pkg), include.only=c(new_names, old))
}

Perhaps detach() could invisibly return the exported names to make this a
tiny bit easier (today it returns NULL):

modify_attach = function(pkg, new_names) {
  if (!startsWith(pkg, "package:")) pkg <- paste0("package:", pkg)
  old <- tryCatch(detach(pkg), error=\(c) character())
  attachNamespace(.rmpkg(pkg), include.only=c(new_names, old))
}

Regardless, I think your suggestion to just point to
detach()+attachNamespace() is reasonable enough, the rare users that care
about this are likely to be able to figure out the rest from there.

On Thu, Apr 11, 2024 at 7:37 AM  wrote:

> On Thu, 11 Apr 2024, Duncan Murdoch wrote:
>
> > On 11/04/2024 7:04 a.m., Martin Maechler wrote:
> >>> Michael Chirico
> >>>  on Mon, 8 Apr 2024 10:19:29 -0700 writes:
> >>
> >>  > Right now, attaching the same package with different
> include.only=
> >> has no
> >>  > effect:
> >>
> >>  > library(Matrix, include.only="fac2sparse")
> >>  > library(Matrix)
> >>  > ls("package:Matrix")
> >>  > # [1] "fac2sparse"
> >>
> >>  > ?library does not cover this case -- what is covered is the
> >> _loading_
> >>  > behavior of repeated calls:
> >>
> >>  >> [library and require] check and update the list of currently
> >> attached
> >>  > packages and do not reload a namespace which is already loaded
> >>
> >>  > But here we're looking at the _attach_ behavior of repeated
> calls.
> >>
> >>  > I am particularly interested in allowing the exports of a
> package to
> >> be
> >>  > built up gradually:
> >>
> >>  > library(Matrix, include.only="fac2sparse")
> >>  > library(Matrix, include.only="isDiagonal") # want:
> >> ls("package:Matrix") -->
> >>  > c("fac2sparse", "isDiagonal")
> >>  > ...
> >>
> >>  > It seems quite hard to accomplish this at the moment. Is the
> >> behavior to
> >>  > ignore new inclusions intentional? Could there be an argument to
> get
> >>  > different behavior?
> >>
> >> As you did not get an answer yet, ..., some remarks by an
> >> R-corer who has tweaked library() behavior in the past :
> >>
> >> - The `include.only = *` argument to library() has been a
> >>*relatively* recent addition {given the 25+ years of R history}:
> >>
> >>It was part of the extensive new features by Luke Tierney for
> >>R 3.6.0  [r76248 | luke | 2019-03-18 17:29:35 +0100], with NEWS entry
> >>
> >>  • library() and require() now allow more control over handling
> >>search path conflicts when packages are attached. The policy is
> >>controlled by the new conflicts.policy option.
> >>
> >> - I haven't seen these (then) new features been used much,
> unfortunately,
> >>also not from R-core members, but I'd be happy to be told a
> different
> >> story.
> >>
> >> For the above reasons, it could well be that the current
> >> implementation {of these features} has not been exercised a lot
> >> yet, and limitations as you found them haven't been noticed yet,
> >> or at least not noticed on the public R mailing lists, nor
> >> otherwise by R-core (?).
> >>
> >> Your implicitly proposed new feature (or even *changed*
> >> default behavior) seems to make sense to me -- but as alluded
> >> to, above, I haven't been a conscious user of any
> >> 'library(.., include.only = *)' till now.
> >
> > I don't think it makes sense.  I would assume that
> >
> >  library(Matrix, include.only="isDiagonal")
> >
> > implies that only `isDiagonal` ends up on the search path, i.e.
> > "include.only" means "include only", not "include in addition to
> whatever
> > else has already been attached".
> >
> > I think a far better approach to solve Michael's problem is simply to use
> >
> >  fac2sparse <- Matrix::fac2sparse
> >  isDiagonal <- Matrix::isDiagonal
> >
> > instead of messing around with the user's search list, which may have
> been
> > intentionally set to include only one of those.
> >
> > So I'd suggest changing the docs to say
> >
> > 

Re: [Rd] [External] Re: Repeated library() of one package with different include.only= entries

2024-04-11 Thread luke-tierney--- via R-devel

On Thu, 11 Apr 2024, Duncan Murdoch wrote:


On 11/04/2024 7:04 a.m., Martin Maechler wrote:

Michael Chirico
 on Mon, 8 Apr 2024 10:19:29 -0700 writes:


 > Right now, attaching the same package with different include.only= 
has no

 > effect:

 > library(Matrix, include.only="fac2sparse")
 > library(Matrix)
 > ls("package:Matrix")
 > # [1] "fac2sparse"

 > ?library does not cover this case -- what is covered is the 
_loading_

 > behavior of repeated calls:

 >> [library and require] check and update the list of currently 
attached

 > packages and do not reload a namespace which is already loaded

 > But here we're looking at the _attach_ behavior of repeated calls.

 > I am particularly interested in allowing the exports of a package to 
be

 > built up gradually:

 > library(Matrix, include.only="fac2sparse")
 > library(Matrix, include.only="isDiagonal") # want: 
ls("package:Matrix") -->

 > c("fac2sparse", "isDiagonal")
 > ...

 > It seems quite hard to accomplish this at the moment. Is the 
behavior to

 > ignore new inclusions intentional? Could there be an argument to get
 > different behavior?

As you did not get an answer yet, ..., some remarks by an
R-corer who has tweaked library() behavior in the past :

- The `include.only = *` argument to library() has been a
   *relatively* recent addition {given the 25+ years of R history}:

   It was part of the extensive new features by Luke Tierney for
   R 3.6.0  [r76248 | luke | 2019-03-18 17:29:35 +0100], with NEWS entry

 • library() and require() now allow more control over handling
   search path conflicts when packages are attached. The policy is
   controlled by the new conflicts.policy option.

- I haven't seen these (then) new features been used much, unfortunately,
   also not from R-core members, but I'd be happy to be told a different 
story.


For the above reasons, it could well be that the current
implementation {of these features} has not been exercised a lot
yet, and limitations as you found them haven't been noticed yet,
or at least not noticed on the public R mailing lists, nor
otherwise by R-core (?).

Your implicitly proposed new feature (or even *changed*
default behavior) seems to make sense to me -- but as alluded
to, above, I haven't been a conscious user of any
'library(.., include.only = *)' till now.


I don't think it makes sense.  I would assume that

 library(Matrix, include.only="isDiagonal")

implies that only `isDiagonal` ends up on the search path, i.e. 
"include.only" means "include only", not "include in addition to whatever 
else has already been attached".


I think a far better approach to solve Michael's problem is simply to use

 fac2sparse <- Matrix::fac2sparse
 isDiagonal <- Matrix::isDiagonal

instead of messing around with the user's search list, which may have been 
intentionally set to include only one of those.


So I'd suggest changing the docs to say

"[library and require] check and update the list of currently attached
packages and do not reload a namespace which is already loaded.  If a package 
is already attached, no change will be made."


?library could also mention using detach() followed by library() or
attachNamespace() with a new include.only specification.

Best,

luke



Duncan Murdoch

__
R-devel@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 andFax:   319-335-3017
   Actuarial Science
241 Schaeffer Hall  email:   luke-tier...@uiowa.edu
Iowa City, IA 52242 WWW:  http://www.stat.uiowa.edu/
__
R-devel@r-project.org mailing list
https://stat.ethz.ch/mailman/listinfo/r-devel