joshuaulrich / xts

Extensible time series class that provides uniform handling of many R time series classes by extending zoo.
http://joshuaulrich.github.io/xts/
GNU General Public License v2.0
220 stars 71 forks source link

Add ylab to plot.xts #334

Closed jaymon0703 closed 4 years ago

jaymon0703 commented 4 years ago

Few things worth mentioning:

  1. Tried to do this in chart.lines() but ylab was not passing through
  2. Need a solution for when multi.panel is not NULL, perhaps a character vector of ylab strings?
  3. Needed the plot printed before i could add title() hence cs becomes plot(cs)

Simple test is add ylab argument to plot.xts()

See #333

Please review the contributing guide before submitting your pull request. Please pay special attention to the pull request and commit message sections. Thanks for your contribution and interest in the project!

jaymon0703 commented 4 years ago

I should mention i did try sub and xlab args, but they overlap the x-axis labels so would not work without some extra effort which im not sure is warranted.

joshuaulrich commented 4 years ago

This seems to work for me. Can you try it and see if it does what you expect?

diff --git a/R/plot.R b/R/plot.R
index 01474a3..11b6fcc 100644
--- a/R/plot.R
+++ b/R/plot.R
@@ -286,6 +286,7 @@ plot.xts <- function(x,
   cs$Env$column_names <- colnames(x)
   cs$Env$nobs <- NROW(cs$Env$xdata)
   cs$Env$main <- main
+  cs$Env$ylab <- if (hasArg("ylab")) eval.parent(plot.call$ylab) else ""

   # Set xlim using the raw returns data passed into function
   # xlim can be based on observations or time
@@ -411,6 +412,10 @@ plot.xts <- function(x,
                              col=theme$labels, srt=theme$srt, offset=1, pos=4,
                              cex=theme$cex.axis, xpd=TRUE)))
   }
+
+  # ylab
+  exp <- c(exp, expression(title(ylab = ylab[1], mgp = c(1, 1, 0))))
+
   cs$add(exp, env=cs$Env, expr=TRUE)

   # add main series
jaymon0703 commented 4 years ago

Thanks Josh. Is there a way for me to pull your changes, or must i add them manually?

joshuaulrich commented 4 years ago

I can push them to your branch if they look good to you.

jaymon0703 commented 4 years ago

I can push them to your branch if they look good to you.

Thanks Josh, it works. Feel free to push.

joshuaulrich commented 4 years ago

I just noticed your comment about how to handle when multi.panel != NULL. Did you try that with my change? Do you think we should try adding that now, or wait to handle it in a separate issue/PR? My slight preference would be to merge this and deal with the multi.panel case later.

jaymon0703 commented 4 years ago

Later is fine. I have pulled your changes, thanks. It seems the PR is updated with your changes. Is there anything left for me to do on the PR?

joshuaulrich commented 4 years ago

Nothing more for you to do. I just needed to merge. Thanks for this suggestion and feedback!

jaymon0703 commented 4 years ago

Thank you Josh.

jaymon0703 commented 4 years ago

The idea came from some algoTCA charts we plot with xts in blotter. Now we can show the metric for the y-axis. Thanks again. image