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

trying another approach for custom grid #386

Merged
merged 3 commits into from
Jul 8, 2016

Conversation

jordansread
Copy link
Member

@jordansread
Copy link
Member Author

@lindsaycarr there is redundant code in here, but this was kind of what I had in mind

@lindsayplatt
Copy link

all the 👍 this is so much cleaner than what I was implementing (and I branched, so easy to delete!)

@jordansread
Copy link
Member Author

Want to take this and streamline it (get rid of the redundancy and other stuff) and add your tests? Curious if it passes your new tests - I didn't try them.

@lindsayplatt
Copy link

Yes, I can do that. Should I make a branch of yours and do a pull request to your fork when ready?

}
abline(v=at[[as.x_side_name(view.name)]], remove_field(grid.args, "equilogs"))
} else {
do.call(graphics::grid, args = append(list('ny'=NA), remove_field(grid.args, "ny")))
Copy link
Member

Choose a reason for hiding this comment

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

I think we still need to account for the case where either nx=NA or nx=0, and capture our custom tick marks. Maybe?

Copy link
Member

Choose a reason for hiding this comment

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

nevermind, it's by side...

@jordansread
Copy link
Member Author

@lindsaycarr feel free to pull down this PR (e.g., git pull https://github.com/jread-usgs/gsplot.git view_filter) and do a new PR against that branch on my repo
https://github.com/jread-usgs/gsplot/tree/view_filter

}
abline(h=at[[as.y_side_name(view.name)]], remove_field(grid.args, "equilogs"))
} else {
do.call(graphics::grid, args = append(list('nx'=NA), remove_field(grid.args, "nx")))
Copy link
Member Author

Choose a reason for hiding this comment

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

Also, I see now that this could be the simpler:

grid('nx'=NA, remove_field(grid.args, "nx"))

for some reason that doesn't work if you use graphics::grid('nx'=NA, remove_field(grid.args, "nx")), which is what I tried first

@jordansread
Copy link
Member Author

@lindsaycarr got your's in here. I'll leave the merge up to you if you are 👍

@jordansread jordansread mentioned this pull request Jul 8, 2016
@coveralls
Copy link

Coverage Status

Coverage decreased (-0.6%) to 77.338% when pulling c1ca5de on jread-usgs:view_filter into fd18d5e on USGS-R:master.

@ldecicco-USGS ldecicco-USGS merged commit 0cb2867 into USGS-R:master Jul 8, 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.

4 participants