Re: [Rd] How to modify dots and dispatch NextMethod
The example is invoking NextMethod via an anonymous function, which is
not allowed (see documentation for NextMethod). Normally one gets a
runtime error "'NextMethod' called from an anonymous function", but not
here as the anonymous function is called via do.call. I will fix so that
there is a runtime error in this case as well, thanks for uncovering
this problem.
I don't think there is a way to replace (unnamed) arguments in dots for
NextMethod.
Tomas
On 02/21/2018 02:16 PM, Iñaki Úcar wrote:
I've set up a repo with a reproducible example of the issue described
in my last email:
https://github.com/Enchufa2/dispatchS3dots
Iñaki
2018-02-20 19:33 GMT+01:00 Iñaki Úcar :
Hi all,
Not sure if this belongs to R-devel or R-package-devel. Anyways...
Suppose we have objects of class c("foo", "bar"), and there are two S3
methods c.foo, c.bar. In c.foo, I'm trying to modify the dots and
forward the dispatch using NextMethod without any success. This is
what I've tried so far:
c.foo <- function(..., recursive=FALSE) {
dots <- list(...)
# inspect and modify dots
# ...
do.call(
function(..., recursive=FALSE) structure(NextMethod("c"), class="foo"),
c(dots, recursive=recursive)
)
}
foobar <- 1
class(foobar) <- c("foo", "bar")
c(foobar, foobar)
Error: C stack usage 7970788 is too close to the limit
There's recursion (!). But the funniest thing is that if c.foo is
exported by one package and c.bar is exported by another one, there's
no recursion, but c.bar is never called (!!). Why is the same code
behaving in a different way depending on whether these functions are
defined in the .GlobalEnv or in two attached packages? (BTW,
isS3method() is TRUE, for c.foo and c.bar in both cases).
I'm blocked here. Am I missing something? Is there a way of doing
this? Thanks in advance.
Regards,
Iñaki
__
R-devel@r-project.org mailing list
https://stat.ethz.ch/mailman/listinfo/r-devel
Re: [Rd] How to modify dots and dispatch NextMethod
2018-02-22 10:29 GMT+01:00 Tomas Kalibera : > > The example is invoking NextMethod via an anonymous function, which is not > allowed (see documentation for NextMethod). Thanks for your response. I definitely missed that bit. > Normally one gets a runtime > error "'NextMethod' called from an anonymous function", but not here as the > anonymous function is called via do.call. I will fix so that there is a > runtime error in this case as well, thanks for uncovering this problem. Then I did well chosing this list! Please also note that you could take that anonymous function out of the method and name it, and the behaviour would be the same. So maybe this case should issue an error too. > I don't think there is a way to replace (unnamed) arguments in dots for > NextMethod. That's a pity. IMHO, it should be some mechanism for that, but dots are special in inscrutable ways. Anyway, for anyone insterested, I found a workaround: https://github.com/Enchufa2/dispatchS3dots#workaround > > Tomas > > Iñaki __ R-devel@r-project.org mailing list https://stat.ethz.ch/mailman/listinfo/r-devel
Re: [Rd] Duplicate column names created by base::merge() when by.x has the same name as a column in y
> Gabriel Becker
> on Wed, 21 Feb 2018 07:11:44 -0800 writes:
> Hi all,
> For the record this approach isnt 100% backwards compatible, because
> names(mergeddf) will e incompatibly different. Thatx why i claimed
> bakcwards compatable-ish
exactly.
> That said its still worth considering imho because of the reasons stated
> (and honestly one particular simple reading of the docs might suggest that
> this was thr intended behavior all along). Im not a member of Rcore
through
> so i cant do said considering myself.
I agree with Scott, Frederik and you that this changes seems
worth considering.
As Duncan Murdoch has mentioned, this alone may not be
sufficient.
In addition to your proposed patch (which I have simplified, not
using intersection() but working with underlying match()
directly), it is little work to introduce an extra argument, I'm
calling 'no.dups = TRUE' which when set to false would mirror
current R's behavior... and documenting it, then also documents the
new behavior (to some extent).
My plan is to commit it soonish ;-)
Martin
> Best,
> ~G
> On Feb 20, 2018 7:15 PM, wrote:
> Hi Scott,
> I tried the new patch and can confirm that it has the advertised
> behavior on a couple of test cases. I think it makes sense to apply
> it, because any existing code which refers to a second duplicate
> data.frame column by name is already broken, while if the reference is
> by numerical index then changing the column name shouldn't break it.
> I don't know if you need to update the documentation as part of your
> patch, or if whoever applies it would be happy to do that. Somebody
> from R core want to weigh in on this?
> I attach a file with the test example from your original email as well
> as a second test case I added with two "by" columns.
> Thanks,
> Frederick
> On Wed, Feb 21, 2018 at 10:06:21AM +1100, Scott Ritchie wrote:
>> Hi Frederick,
>>
>> It looks like I didn't overwrite the patch.diff file after the last
edits.
>> Here's the correct patch (attached and copied below):
>>
>> Index: src/library/base/R/merge.R
>> ===
>> --- src/library/base/R/merge.R (revision 74280)
>> +++ src/library/base/R/merge.R (working copy)
>> @@ -157,6 +157,14 @@
>> }
>>
>> if(has.common.nms) names(y) <- nm.y
>> +## If by.x %in% names(y) then duplicate column names still
arise,
>> +## apply suffixes to just y - this keeps backwards compatibility
>> +## when referring to by.x in the resulting data.frame
>> +dupe.keyx <- intersect(nm.by, names(y))
>> +if(length(dupe.keyx)) {
>> + if(nzchar(suffixes[2L]))
>> +names(y)[match(dupe.keyx, names(y), 0L)] <- paste(dupe.keyx,
>> suffixes[2L], sep="")
>> +}
>> nm <- c(names(x), names(y))
>> if(any(d <- duplicated(nm)))
>> if(sum(d) > 1L)
>>
>> Best,
>>
>> Scott
>>
>> On 21 February 2018 at 08:23, wrote:
>>
>> > Hi Scott,
>> >
>> > I think that's a good idea and I tried your patch on my copy of the
>> > repository. But it looks to me like the recent patch is identical to
>> > the previous one, can you confirm this?
>> >
>> > Frederick
>> >
>> > On Mon, Feb 19, 2018 at 07:19:32AM +1100, Scott Ritchie wrote:
>> > > Thanks Gabriel,
>> > >
>> > > I think your suggested approach is 100% backwards compatible
>> > >
>> > > Currently in the case of duplicate column names only the first can be
>> > > indexed by its name. This will always be the column appearing in
by.x,
>> > > meaning the column in y with the same name cannot be accessed.
> Appending
>> > > ".y" (suffixes[2L]) to this column means it can now be accessed,
while
>> > > keeping the current behaviour of making the key columns always
> accessible
>> > > by using the names provided to by.x.
>> > >
>> > > I've attached a new patch that has this behaviour.
>> > >
>> > > Best,
>> > >
>> > > Scott
>> > >
>> > > On 19 February 2018 at 05:08, Gabriel Becker
>> > wrote:
>> > >
>> > > > It seems like there is a way that is backwards compatible-ish in
the
>> > sense
>> > > > mentioned and still has the (arguably, but a good argument I think)
>> > better
>> > > > behavior:
>> > > >
>> > > > if by.x is 'name', (AND by.y is not also 'name'), then x's 'name'
>> > column
>> > > > is called name and y's 'name' column (not used int he merge) is
>> > changed to
>> > > > name.y.
>> > > >
>> > > > Now of course this would still change output, but it would change
> it to
>> > > > something I think would be better, while retaining the 'merge
> columns
>> > > > reta
Re: [Rd] How to modify dots and dispatch NextMethod
On 02/22/2018 12:07 PM, Iñaki Úcar wrote: 2018-02-22 10:29 GMT+01:00 Tomas Kalibera : The example is invoking NextMethod via an anonymous function, which is not allowed (see documentation for NextMethod). Thanks for your response. I definitely missed that bit. Normally one gets a runtime error "'NextMethod' called from an anonymous function", but not here as the anonymous function is called via do.call. I will fix so that there is a runtime error in this case as well, thanks for uncovering this problem. Then I did well chosing this list! Please also note that you could take that anonymous function out of the method and name it, and the behaviour would be the same. So maybe this case should issue an error too. I am not sure I understand how, but if you find a way to bypass the new check for an anonymous function (I intend to commit tomorrow), I will be happy to have a look if you provide a reproducible example. I don't think there is a way to replace (unnamed) arguments in dots for NextMethod. That's a pity. IMHO, it should be some mechanism for that, but dots are special in inscrutable ways. Anyway, for anyone insterested, I found a workaround: https://github.com/Enchufa2/dispatchS3dots#workaround Even though technically this won't be too hard, I don't think NextMethod should be made any more complex than it is now. There should always be a way to implement special dispatch scenarios in R and your workaround shows it is possible specifically in your scenario. Tomas Tomas Iñaki __ R-devel@r-project.org mailing list https://stat.ethz.ch/mailman/listinfo/r-devel
Re: [Rd] How to modify dots and dispatch NextMethod
2018-02-22 12:39 GMT+01:00 Tomas Kalibera :
> On 02/22/2018 12:07 PM, Iñaki Úcar wrote:
>>
>> 2018-02-22 10:29 GMT+01:00 Tomas Kalibera :
>>>
>>> The example is invoking NextMethod via an anonymous function, which is
>>> not
>>> allowed (see documentation for NextMethod).
>>
>> Thanks for your response. I definitely missed that bit.
>>
>>> Normally one gets a runtime
>>> error "'NextMethod' called from an anonymous function", but not here as
>>> the
>>> anonymous function is called via do.call. I will fix so that there is a
>>> runtime error in this case as well, thanks for uncovering this problem.
>>
>> Then I did well chosing this list! Please also note that you could
>> take that anonymous function out of the method and name it, and the
>> behaviour would be the same. So maybe this case should issue an error
>> too.
>
> I am not sure I understand how, but if you find a way to bypass the new
> check for an anonymous function (I intend to commit tomorrow), I will be
> happy to have a look if you provide a reproducible example.
I meant with a named function inside do.call, instead of an anonymous
one. For example:
c.foo <- function(..., recursive=FALSE) {
message("calling c.foo...")
dots <- list(...)
# inspect and modify dots; for example:
if (length(dots > 1))
dots[[2]] <- 2
do.call(
c.foo.proxy,
c(dots, recursive=recursive)
)
}
c.foo.proxy <- function(..., recursive=FALSE)
structure(NextMethod("c"), class="foo")
Right now, the effect of the code above is the same as with the
anonymous function. Shouldn't it issue a similar error then?
>>>
>>> I don't think there is a way to replace (unnamed) arguments in dots for
>>> NextMethod.
>>
>> That's a pity. IMHO, it should be some mechanism for that, but dots
>> are special in inscrutable ways.
>>
>> Anyway, for anyone insterested, I found a workaround:
>>
>> https://github.com/Enchufa2/dispatchS3dots#workaround
>
> Even though technically this won't be too hard, I don't think NextMethod
> should be made any more complex than it is now. There should always be a way
> to implement special dispatch scenarios in R and your workaround shows it is
> possible specifically in your scenario.
My only concern about this workaround is that it triggers the dispatch
stack again from the beginning of the class hierarchy, which seems not
very elegant nor efficient.
>
> Tomas
>
Iñaki
__
R-devel@r-project.org mailing list
https://stat.ethz.ch/mailman/listinfo/r-devel
Re: [Rd] How to modify dots and dispatch NextMethod
On 02/22/2018 02:31 PM, Iñaki Úcar wrote:
2018-02-22 12:39 GMT+01:00 Tomas Kalibera :
On 02/22/2018 12:07 PM, Iñaki Úcar wrote:
2018-02-22 10:29 GMT+01:00 Tomas Kalibera :
The example is invoking NextMethod via an anonymous function, which is
not
allowed (see documentation for NextMethod).
Thanks for your response. I definitely missed that bit.
Normally one gets a runtime
error "'NextMethod' called from an anonymous function", but not here as
the
anonymous function is called via do.call. I will fix so that there is a
runtime error in this case as well, thanks for uncovering this problem.
Then I did well chosing this list! Please also note that you could
take that anonymous function out of the method and name it, and the
behaviour would be the same. So maybe this case should issue an error
too.
I am not sure I understand how, but if you find a way to bypass the new
check for an anonymous function (I intend to commit tomorrow), I will be
happy to have a look if you provide a reproducible example.
I meant with a named function inside do.call, instead of an anonymous
one. For example:
c.foo <- function(..., recursive=FALSE) {
message("calling c.foo...")
dots <- list(...)
# inspect and modify dots; for example:
if (length(dots > 1))
dots[[2]] <- 2
do.call(
c.foo.proxy,
c(dots, recursive=recursive)
)
}
c.foo.proxy <- function(..., recursive=FALSE)
structure(NextMethod("c"), class="foo")
Right now, the effect of the code above is the same as with the
anonymous function. Shouldn't it issue a similar error then?
Yes, it will also result in runtime error after the change is committed:
calling c.foo...
Error in NextMethod("c") : 'NextMethod' called from an anonymous function
I don't think there is a way to replace (unnamed) arguments in dots for
NextMethod.
That's a pity. IMHO, it should be some mechanism for that, but dots
are special in inscrutable ways.
Anyway, for anyone insterested, I found a workaround:
https://github.com/Enchufa2/dispatchS3dots#workaround
Even though technically this won't be too hard, I don't think NextMethod
should be made any more complex than it is now. There should always be a way
to implement special dispatch scenarios in R and your workaround shows it is
possible specifically in your scenario.
My only concern about this workaround is that it triggers the dispatch
stack again from the beginning of the class hierarchy, which seems not
very elegant nor efficient.
There may be a more elegant way, but that'd be a question for R-help and
it might be worth giving a broader context for what you want to achieve.
Also please note that S3 dispatch is done on the first argument, and c()
gives no special meaning to its first argument, what if e.g. the second
argument is of class "foo" but the first is not - is S3/NextMethod
really a good fit here?
Tomas
Tomas
Iñaki
__
R-devel@r-project.org mailing list
https://stat.ethz.ch/mailman/listinfo/r-devel
Re: [Rd] Duplicate column names created by base::merge() when by.x has the same name as a column in y
Thanks Martin!
Can you clarify the functionality of the 'no.dups' argument so I can change
my patch to `data.table:::merge.data.table` accordingly?
- When `no.dups=TRUE` will the suffix to the by.x column name? Or will it
take the functionality of the second functionality where only the column in
y has the suffix added?
- When `no.dups=FALSE` will the output be the same as it currently (no
suffix added to either column)? Or will add the suffix to the column in y?
Best,
Scott
On 22 February 2018 at 22:31, Martin Maechler
wrote:
> > Gabriel Becker
> > on Wed, 21 Feb 2018 07:11:44 -0800 writes:
>
> > Hi all,
> > For the record this approach isnt 100% backwards compatible, because
> > names(mergeddf) will e incompatibly different. Thatx why i claimed
> > bakcwards compatable-ish
>
> exactly.
>
> > That said its still worth considering imho because of the reasons
> stated
> > (and honestly one particular simple reading of the docs might
> suggest that
> > this was thr intended behavior all along). Im not a member of Rcore
> through
> > so i cant do said considering myself.
>
> I agree with Scott, Frederik and you that this changes seems
> worth considering.
> As Duncan Murdoch has mentioned, this alone may not be
> sufficient.
>
> In addition to your proposed patch (which I have simplified, not
> using intersection() but working with underlying match()
> directly), it is little work to introduce an extra argument, I'm
> calling 'no.dups = TRUE' which when set to false would mirror
> current R's behavior... and documenting it, then also documents the
> new behavior (to some extent).
>
> My plan is to commit it soonish ;-)
> Martin
>
> > Best,
> > ~G
>
> > On Feb 20, 2018 7:15 PM, wrote:
>
> > Hi Scott,
>
> > I tried the new patch and can confirm that it has the advertised
> > behavior on a couple of test cases. I think it makes sense to apply
> > it, because any existing code which refers to a second duplicate
> > data.frame column by name is already broken, while if the reference
> is
> > by numerical index then changing the column name shouldn't break it.
>
> > I don't know if you need to update the documentation as part of your
> > patch, or if whoever applies it would be happy to do that. Somebody
> > from R core want to weigh in on this?
>
> > I attach a file with the test example from your original email as
> well
> > as a second test case I added with two "by" columns.
>
> > Thanks,
>
> > Frederick
>
> > On Wed, Feb 21, 2018 at 10:06:21AM +1100, Scott Ritchie wrote:
> >> Hi Frederick,
> >>
> >> It looks like I didn't overwrite the patch.diff file after the last
> edits.
> >> Here's the correct patch (attached and copied below):
> >>
> >> Index: src/library/base/R/merge.R
> >> ===
> >> --- src/library/base/R/merge.R (revision 74280)
> >> +++ src/library/base/R/merge.R (working copy)
> >> @@ -157,6 +157,14 @@
> >> }
> >>
> >> if(has.common.nms) names(y) <- nm.y
> >> +## If by.x %in% names(y) then duplicate column names still
> arise,
> >> +## apply suffixes to just y - this keeps backwards
> compatibility
> >> +## when referring to by.x in the resulting data.frame
> >> +dupe.keyx <- intersect(nm.by, names(y))
> >> +if(length(dupe.keyx)) {
> >> + if(nzchar(suffixes[2L]))
> >> +names(y)[match(dupe.keyx, names(y), 0L)] <-
> paste(dupe.keyx,
> >> suffixes[2L], sep="")
> >> +}
> >> nm <- c(names(x), names(y))
> >> if(any(d <- duplicated(nm)))
> >> if(sum(d) > 1L)
> >>
> >> Best,
> >>
> >> Scott
> >>
> >> On 21 February 2018 at 08:23, wrote:
> >>
> >> > Hi Scott,
> >> >
> >> > I think that's a good idea and I tried your patch on my copy of
> the
> >> > repository. But it looks to me like the recent patch is identical
> to
> >> > the previous one, can you confirm this?
> >> >
> >> > Frederick
> >> >
> >> > On Mon, Feb 19, 2018 at 07:19:32AM +1100, Scott Ritchie wrote:
> >> > > Thanks Gabriel,
> >> > >
> >> > > I think your suggested approach is 100% backwards compatible
> >> > >
> >> > > Currently in the case of duplicate column names only the first
> can be
> >> > > indexed by its name. This will always be the column appearing
> in by.x,
> >> > > meaning the column in y with the same name cannot be accessed.
> > Appending
> >> > > ".y" (suffixes[2L]) to this column means it can now be
> accessed, while
> >> > > keeping the current behaviour of making the key columns always
> > accessible
> >> > > by using the names provided to by.x.
> >> > >
> >> > > I've attached a new patch that has this behaviour.
> >> > >
> >> > > Best,
> >> > >
[Rd] Bug in installing rgdal
*@* *BLAS: /usr/lib/libblas/libblas.so.3.6.0LAPACK: /usr/lib/lapack/liblapack.so.3.6.0locale: [1] LC_CTYPE=en_IN.UTF-8 LC_NUMERIC=C LC_TIME=en_IN.UTF-8[4] LC_COLLATE=en_IN.UTF-8 LC_MONETARY=en_IN.UTF-8 LC_MESSAGES=en_IN.UTF-8[7] LC_PAPER=en_IN.UTF-8 LC_NAME=C LC_ADDRESS=C [10] LC_TELEPHONE=C LC_MEASUREMENT=en_IN.UTF-8 LC_IDENTIFICATION=C * Can someone please guide me further? Regards, -- Mohit Kumar Research Engineer in Data Science Trivedi Center for Political Data Ashoka University +91-9703840175 [[alternative HTML version deleted]] __ R-devel@r-project.org mailing list https://stat.ethz.ch/mailman/listinfo/r-devel
