Paul Johnson
2013-Apr-18 03:39 UTC
[Rd] Patch proposal for R style consistency (concerning deparse.c)
Hello, everybody.
I recognize I'm asking you to deal with a not-very-important problem.
But its important to me :)
I've noticed a little inconsistency in the print.function() output. I
traced the matter to deparse.c, for which I attach a patch that
addresses 3 separate things. There's one important thing that I'm
willing to stand behind and 2 litter things that I think would be
good, but won't argue too much about. I ask if this is a legitimate
wishlist item in your bug tracker.
Here are the changes I'm trying to achieve.
1. I want this:
} else {
and not the current
}
else {
2 & 3. I want to omit space after if and for. Since if and for are
functions in R, not keywords, I suggest that there should not be a
space before the opening parentheses.
Item 1 brings the output into line with the advice of the help page
for "if". The output looks more like the a randomly selected R file
from the R source or the recommended packages. I'm teaching new users
that they should study the R source code itself as a guide for
programming (http://pj.freefaculty.org/R/Rstyle.pdf). My argument
there would go more smoothly if print.function obeyed the "} else {"
policy that most of your source code and packages adhere to. We do see
r-help questions about the problem that user code can't be run
line-by-line, even when it is styled like the print.function() output.
As you know, this is discussed in R Extensions Section 3.1 on tidying
R code. In my opinion, the current approach makes code less tidy,
rather than more.
Consider an excerpt from the top of predict.glm.R. This R code is
correctly formatted, it is consistent with most of the rest of R:
if (!se.fit) {
## No standard errors
if(missing(newdata)) {
pred <- switch(type,
link = object$linear.predictors,
response = object$fitted.values,
terms = predict.lm(object, se.fit=se.fit,
scale = 1, type="terms", terms=terms)
)
if(!is.null(na.act)) pred <- napredict(na.act, pred)
} else {
But the output from '> predict.glm' is not as good:
if (!se.fit) {
if (missing(newdata)) {
pred <- switch(type, link = object$linear.predictors,
response = object$fitted.values, terms = predict.lm(object,
se.fit = se.fit, scale = 1, type = "terms",
terms = terms))
if (!is.null(na.act))
pred <- napredict(na.act, pred)
}
else {
I have applied the attached patch with R 3.0.0. I don't see any damage
from it, just more tidier output:
> predict.glm
function (object, newdata = NULL, type = c("link",
"response",
"terms"), se.fit = FALSE, dispersion = NULL, terms = NULL,
na.action = na.pass, ...)
{
type <- match.arg(type)
na.act <- object$na.action
object$na.action <- NULL
if(!se.fit) {
if(missing(newdata)) {
pred <- switch(type, link = object$linear.predictors,
response = object$fitted.values, terms = predict.lm(object,
se.fit = se.fit, scale = 1, type = "terms",
terms = terms))
if(!is.null(na.act))
pred <- napredict(na.act, pred)
} else {
pred <- predict.lm(object, newdata, se.fit, scale = 1,
type = ifelse(type == "link", "response",
type),
terms = terms, na.action = na.action)
switch(type, response = {
pred <- family(object)$linkinv(pred)
}, link = , terms = )
}
} else {
Of course, you know much better than I do what else might break as a
result of this change.
In case the email list processor scrubs the patch, please try here:
http://pj.freefaculty.org/scraps/deparse-pj-20130417.patch
pj
--
Paul E. Johnson
Professor, Political Science Assoc. Director
1541 Lilac Lane, Room 504 Center for Research Methods
University of Kansas University of Kansas
http://pj.freefaculty.org http://quant.ku.edu
peter dalgaard
2013-Apr-18 08:05 UTC
[Rd] Patch proposal for R style consistency (concerning deparse.c)
On Apr 18, 2013, at 05:39 , Paul Johnson wrote:> 2 & 3. I want to omit space after if and for. Since if and for are > functions in R, not keywords, I suggest that there should not be a > space before the opening parentheses.Wrong. They are part of language constructs (and they _are_ keywords, not names, that's why ?for won't work). The function calls are `if`(fee, {foo}, {fie}) and something rebarbative for `for`(....). Besides, both constructs are harder to read without the spaces. -- Peter Dalgaard, Professor Center for Statistics, Copenhagen Business School Solbjerg Plads 3, 2000 Frederiksberg, Denmark Phone: (+45)38153501 Email: pd.mes at cbs.dk Priv: PDalgd at gmail.com
Terry Therneau
2013-May-02 12:26 UTC
[Rd] Patch proposal for R style consistency (concerning deparse.c)
I'll be the "anybody" to argue that
} else {
is an ugly kludge which you will never find in my source code. Yes, it's
necessary at the
command line because the parser needs help in guessing when an expression is
finished, but
is only needed in that case. Since I can hardly imagine using else at the
command line
(that many correct characters in a row exceeds my typing skill) it's not an
issue for me.
I most certainly would not inflict this construction on my pupils when teaching
a class,
nor that any break of a long line has to be after "+" but not before,
nor other crutches
for the parser's sake. Let them know about the special case of course, but
don't
sacrifice good coding style the deficiency.
That said, I am completely ambivalent to the result of deparse. Just throwing
up an
objection to the "purity" argument: things were beginning to sound a
bit too bombastic :-).
Terry T.
On 05/02/2013 05:00 AM, r-devel-request at r-project.org
wrote:> I want "} else {". Yihue wants "} else {". And I
have not heard anybody
> say they prefer the other way, unless you interpret Duncan's comment
> "that's nonsense" as a blanket defense of the status quo. But
I don't think
> he meant that. This is a matter of style consistency and avoidance of new
> R-user confusion and error.