Oh but I see now that you've already tried this in your R/AllGenerics.R, sorry for missing that, but that you worry about the following message being disruptive on CRAN:
The following object is masked from 'package:base': qr.X Why would that be? As long as you only define methods for objects that **you** control everything is fine. In other words you're not allowed to define a method for "qr" objects because that method would override base::qr.X(). But the generic itself and the method that you define for your objects don't override anything so should not disrupt anything. H. On 6/15/23 13:51, Hervé Pagès wrote: > > I'd argue that at the root of the problem is that your qr.X() generic > dispatches on all its arguments, including the 'ncol' argument which I > think the dispatch mechanism needs to evaluate **before** dispatch can > actually happen. > > So yes lazy evaluation is a real feature but it does not play well for > arguments of a generic that are involved in the dispatch. > > If you explicitly defined your generic with: > > setGeneric("qr.X", signature="qr") > > you should be fine. > > More generally speaking, it's a good idea to restrict the signature of > a generic to the arguments "that make sense". For unary operations > this is usually the 1st argument, for binary operations the first two > arguments etc... Additional arguments that control the operation like > modiflers, toggles, flags, rng seed, and other parameters, usually > have not place in the signature of the generic. > > H. > > On 6/14/23 20:57, Mikael Jagan wrote: >> Thanks all - yes, I think that Simon's diagnosis ("user error") is >> correct: >> in this situation one should define a reasonable generic function >> explicitly, >> with a call to setGeneric, and not rely on the call inside of >> setMethod ... >> >> But it is still not clear what the way forward should be (for package >> Matrix, >> where we would like to export a method for 'qr.X'). If we do >> nothing, then >> there is the note, already mentioned: >> >> * checking R code for possible problems ... NOTE >> qr.X: no visible binding for global variable ‘R’ >> Undefined global functions or variables: >> R >> >> If we add the following to our R/AllGenerics.R : >> >> setGeneric("qr.X", >> function(qr, complete = FALSE, ncol, ...) >> standardGeneric("qr.X"), >> useAsDefault = function(qr, complete = FALSE, ncol, >> ...) { >> if(missing(ncol)) >> base::qr.X(qr, complete = complete) >> else base::qr.X(qr, complete = complete, ncol = ncol) >> }, >> signature = "qr") >> >> then we get a startup message, which would be quite disruptive on CRAN : >> >> The following object is masked from 'package:base': >> >> qr.X >> >> and if we further add setGenericImplicit("qr.X", restore = (TRUE|FALSE)) >> to our R/zzz.R, then for either value of 'restore' we encounter : >> >> ** testing if installed package can be loaded from temporary >> location >> Error: package or namespace load failed for 'Matrix': >> Function found when exporting methods from the namespace >> 'Matrix' which is not S4 generic: 'qr.X' >> >> Are there possibilities that I have missed? >> >> It seems to me that the best option might be to define an implicit >> generic >> 'qr.X' in methods via '.initImplicitGenerics' in >> methods/R/makeBasicFunsList.R, >> where I see that an implicit generic 'qr.R' is already defined ... ? >> >> The patch pasted below "solves everything", though we'd still have to >> think >> about how to work for versions of R without the patch ... >> >> Mikael >> >> Index: src/library/methods/R/makeBasicFunsList.R >> =================================================================== >> --- src/library/methods/R/makeBasicFunsList.R (revision 84541) >> +++ src/library/methods/R/makeBasicFunsList.R (working copy) >> @@ -263,6 +263,17 @@ >> signature = "qr", where = where) >> setGenericImplicit("qr.R", where, FALSE) >> >> + setGeneric("qr.X", >> + function(qr, complete = FALSE, ncol, ...) >> + standardGeneric("qr.X"), >> + useAsDefault = function(qr, complete = FALSE, ncol, >> ...) { >> + if(missing(ncol)) >> + base::qr.X(qr, complete = complete) >> + else base::qr.X(qr, complete = complete, ncol = >> ncol) >> + }, >> + signature = "qr", where = where) >> + setGenericImplicit("qr.X", where, FALSE) >> + >> ## our toeplitz() only has 'x'; want the generic "here" rather >> than "out there" >> setGeneric("toeplitz", function(x, ...) >> standardGeneric("toeplitz"), >> useAsDefault= function(x, ...) stats::toeplitz(x), >> >> On 2023-06-13 8:01 pm, Simon Urbanek wrote: >>> I agree that this is not an R issue, but rather user error of not >>> defining a proper generic so the check is right. Obviously, defining >>> a generic with implementation-specific ncol default makes no sense >>> at all, it should only be part of the method implementation. If one >>> was to implement the same default behavior in the generic itself >>> (not necessarily a good idea) the default would be ncol = if >>> (complete) nrow(qr.R(qr, TRUE)) else min(dim(qr.R(qr, TRUE))) to not >>> rely on the internals of the implementation. >>> >>> Cheers, >>> Simon >>> >>> >>>> On 14/06/2023, at 6:03 AM, Kasper Daniel Hansen >>>> <kasperdanielhan...@gmail.com> wrote: >>>> >>>> On Sat, Jun 3, 2023 at 11:51 AM Mikael Jagan <jagan...@gmail.com> >>>> wrote: >>>> >>>>> The formals of the newly generic 'qr.X' are inherited from the >>>>> non-generic >>>>> function in the base namespace. Notably, the inherited default >>>>> value of >>>>> formal argument 'ncol' relies on lazy evaluation: >>>>> >>>>>> formals(qr.X)[["ncol"]] >>>>> if (complete) nrow(R) else min(dim(R)) >>>>> >>>>> where 'R' must be defined in the body of any method that might >>>>> evaluate >>>>> 'ncol'. >>>>> >>>> >>>> Perhaps I am misunderstanding something, but I think Mikael's >>>> expectations >>>> about the scoping rules of R are wrong. The enclosing environment >>>> of ncol >>>> is where it was _defined_ not where it is _called_ (apologies if I am >>>> messing up the computer science terminology here). >>>> >>>> This suggests to me that codetools is right. But a more extended >>>> example >>>> would be useful. Perhaps there is something special with setOldClass() >>>> which I am no aware of. >>>> >>>> Also, Bioconductor has 100s of packages with S4 where codetools >>>> works well. >>>> >>>> Kasper >>>> >>>> [[alternative HTML version deleted]] >>>> >>>> ______________________________________________ >>>> R-devel@r-project.org mailing list >>>> https://stat.ethz.ch/mailman/listinfo/r-devel >>>> >>> >> >> ______________________________________________ >> R-devel@r-project.org mailing list >> https://stat.ethz.ch/mailman/listinfo/r-devel > -- > Hervé Pagès > > Bioconductor Core Team > hpages.on.git...@gmail.com -- Hervé Pagès Bioconductor Core Team hpages.on.git...@gmail.com [[alternative HTML version deleted]] ______________________________________________ R-devel@r-project.org mailing list https://stat.ethz.ch/mailman/listinfo/r-devel