Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

draw_axis handling different data types #396

Merged
merged 10 commits into from
Jul 13, 2016

Conversation

lindsayplatt
Copy link

@lindsayplatt lindsayplatt commented Jul 11, 2016

fixing #395

sharing some helper functions with draw_custom_grid, but not quite to the extent #343 is talking about

@lindsayplatt lindsayplatt mentioned this pull request Jul 11, 2016
4 tasks
@coveralls
Copy link

Coverage Status

Coverage increased (+0.1%) to 78.131% when pulling dfa2c0b on lindsaycarr:master into 40dbb30 on USGS-R:master.

@coveralls
Copy link

Coverage Status

Coverage increased (+0.1%) to 78.131% when pulling dfa2c0b on lindsaycarr:master into 40dbb30 on USGS-R:master.

@@ -88,14 +88,22 @@ axis.gsplot <- function(object, ..., n.minor=0, tcl.minor=0.15, reverse=NULL) {

}

draw_axis <- function(axis.args){
draw_axis <- function(object, side.args){
Copy link
Member

@jordansread jordansread Jul 11, 2016

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

aren't side.args redundant w/ object? How about just object and side or side.name?

fun.name <- 'axis'
} else {
fun.name <- paste0('axis.', class(side.lim)[1L])
axis.args$at <- get_axTicks(object, as.side(side.name))
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

can you remove this line? What does it do? I think you want to let the function axis.class figure out the at

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

as.POSIXct and axis.Date don't allow at to be NULL, so I had to define it and then pass it in

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

it will just return Error in as.Date(x) : argument "x" is missing, with no default

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ahh, ok. thanks

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks like axis.args$at doesn't handle

gs <- gsplot() %>% 
     points(some_dates, 1:12) %>% 
     axis(1)
gs

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

in what way?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

geez, sorry. I flipped branches too many times trying to track this down. It does handle that as expected.

I am completely confused why you can give axis all those at values and it still only renders the two extremes. This issue has me totally stumped.

@coveralls
Copy link

Coverage Status

Coverage increased (+0.1%) to 78.15% when pulling ca6f68f on lindsaycarr:master into 40dbb30 on USGS-R:master.

@lindsayplatt
Copy link
Author

lindsayplatt commented Jul 13, 2016

using pretty() and everything does match base!

@lindsayplatt
Copy link
Author

lindsayplatt commented Jul 13, 2016

Base and gsplot are matching now

Base

some_dates <- seq(as.POSIXct("2010-10-01"), as.POSIXct("2011-09-30"), by="month")
plot(some_dates, 1:12)

image

gsplot

gs <- gsplot() %>% 
  points(some_dates, 1:12)
gs

image

@lindsayplatt
Copy link
Author

Another example:

Base

library(dataRetrieval)
siteNumber <- '04085427'
startDate <- '2012-01-01'
endDate <- '2012-06-30'
pCode <- '00060'
dv <- readNWISdv(siteNumber,pCode, startDate, endDate)

plot(dv$Date, dv$X_00060_00003)

image

gsplot

gsplot() %>%
  points(dv$Date, dv$X_00060_00003)

image

@lindsayplatt
Copy link
Author

Test for this is in the README

see Compatibility with base plotting section

date_vector <- seq(as.Date("2010-10-01"), as.Date("2011-09-30"), by="months")
gs <- gsplot() %>% 
  points(date_vector, 1:12)
gs

points(as.Date("2010-11-15"),2.5)

What the plot would look like before this PR:
image

After this PR:
image

@@ -96,8 +96,7 @@ grid_axTicks <- function(object, side){
if(is.numeric(lim)){
at <- axTicks(side)
} else{
fun <- getFromNamespace(paste0('axis.',class(lim)[1L]),'graphics')
at <- fun(side, x = lim, lwd=0, lwd.ticks=0, labels = FALSE)
at <- Axis(side = side, x = pretty(lim), lwd=0, lwd.ticks=0, labels = FALSE)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ahh, so this is the key change?

Copy link
Author

@lindsayplatt lindsayplatt Jul 13, 2016

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yep - needed pretty() so that it wasn't just passing two values. Then switched to using Axis because that internally handles all of the axis.[class] stuff

@coveralls
Copy link

Coverage Status

Coverage increased (+1.4%) to 79.451% when pulling 23f522c on lindsaycarr:master into 40dbb30 on USGS-R:master.

@lindsayplatt lindsayplatt merged commit f6601bb into USGS-R:master Jul 13, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants