Bug 15329 - Wishlist - Add termplot option to plot vs transformed x (code supplied)
Wishlist - Add termplot option to plot vs transformed x (code supplied)
Status: CLOSED FIXED
Product: R
Classification: Unclassified
Component: I/O
R 3.0.0
All All
: P5 enhancement
Assigned To: R-core
: 15330 (view as bug list)
Depends on:
Blocks:
  Show dependency treegraph
 
Reported: 2013-06-01 09:44 UTC by John Maindonald
Modified: 2013-08-02 13:55 UTC (History)
1 user (show)

See Also:


Attachments
Code, incorporating patches (7.84 KB, application/octet-stream)
2013-06-01 09:44 UTC, John Maindonald
Details
Patch against function source (1.25 KB, patch)
2013-08-01 10:24 UTC, John Maindonald
Details | Diff
Patch to termplot.Rd (revision 63471) (1.57 KB, patch)
2013-08-02 11:18 UTC, John Maindonald
Details | Diff
Patch to termplot.R (revision 63471) (3.23 KB, patch)
2013-08-02 11:21 UTC, John Maindonald
Details | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description John Maindonald 2013-06-01 09:44:43 UTC
Created attachment 1449 [details]
Code, incorporating patches

In a command such as the following
  library(DAAG)
  nihills.lm2 <- lm(log(time) ~ log(dist) + poly(climb,2), data=nihills)
it is often preferable to show log(dist) or poly(time, 2) on the x-axis,
rather than, respectively, dist or time.  Deviations from the fitted curve 
are best judged when the curve is transformed to appear as a line.

The changes required for this are small:

1) Add an argument linearize (or ?), by default FALSE

2) Add, near the beginning, the line:

  if(length(linearize)==1)linearize <- rep(linearize, n.tms)

3) In two places, replace 
                xx <- carrier(cn[[i]])
by
                if(!linearize[i])xx <- carrier(cn[[i]]) else {
                    if(length(cn[[i]])==1)
                        xx <- eval(cn[[i]], data, enclos=pf) else
                    xx <- tms[,i]
                    xlabs[i] <- deparse(cn[[i]])
                }

I attach (i) a patch file
            (ii) code that incorporates the changes
Comment 1 Duncan Murdoch 2013-06-01 12:58:19 UTC
*** Bug 15330 has been marked as a duplicate of this bug. ***
Comment 2 Duncan Murdoch 2013-06-01 13:01:11 UTC
The patch isn't against the source of the function, it's against the deparse.  Please obtain a copy of the source from https://svn.r-project.org/R/trunk/src/library/stats/R/termplot.R and apply the changes there.
Comment 3 John Maindonald 2013-08-01 10:24:25 UTC
Created attachment 1468 [details]
Patch against function source

Apologies that I did not see Duncan's response until now -- the message was somehow hidden!
Comment 4 Duncan Murdoch 2013-08-01 13:36:28 UTC
Could you please rework the patch?  I found the following problems:

- I don't think linearized is the right name.  We aren't linearizing anything, we are either transforming it or not.  

- The labels look as though they'd be messed up in the default case if some terms were not transformed.

- The changes should be made within the carrier() function, we shouldn't have duplicated code from that function inserted into the main function in two places.  (It will likely need more parameters).

- Please also patch the man page, including an example that mixes transformed and untransformed terms.

- Please state at the top of the patch file exactly which revision of which file is being patched.  svn diff creates a nice patch file in this format.
Comment 5 John Maindonald 2013-08-02 11:18:59 UTC
Created attachment 1469 [details]
Patch to termplot.Rd (revision 63471)
Comment 6 John Maindonald 2013-08-02 11:21:18 UTC
Created attachment 1470 [details]
Patch to termplot.R (revision 63471)
Comment 7 John Maindonald 2013-08-02 11:27:18 UTC
(In reply to comment #4)
> Could you please rework the patch?  I found the following problems:
> 
> - I don't think linearized is the right name.  We aren't linearizing anything,
> we are either transforming it or not.  

I prefer linearize -- one is linearizing the model fit to the partial residuals.
I have however changed the argument to transform.x

> - The labels look as though they'd be messed up in the default case if some
> terms were not transformed.

The labels were as far as I could see OK (e.g., dist becomes log(dist).  However,
with the changes to carrier(), the way of handling this has changed.

> - The changes should be made within the carrier() function, we shouldn't have
> duplicated code from that function inserted into the main function in two
> places.  (It will likely need more parameters).

Done

> - Please also patch the man page, including an example that mixes transformed
> and untransformed terms.

Done

> - Please state at the top of the patch file exactly which revision of which
> file is being patched.  svn diff creates a nice patch file in this format.

Done.  I'd not been aware of the clever tricks that are possible by using svn
to mirror files on my own system.
Comment 8 Duncan Murdoch 2013-08-02 13:55:11 UTC
Now committed (with minor changes) to R-devel, soon to R-patched.