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.glmfunction (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.