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

preparation for new 'grid' unit implementation #12

Merged
merged 4 commits into from
Nov 11, 2019
Merged

Conversation

pmur002
Copy link
Contributor

@pmur002 pmur002 commented Nov 11, 2019

Hi

Thomas Lin Pedersen and I are working on a faster implementation of units in 'grid' (thomasp85/grid#14).
These changes break 'hexbin' because of the setOldClass() statement in hexViewport.R.
This PR suggests a fix that will still work on current R releases (and R-devel), but will also accommodate the new 'grid' units.
Would you consider incorporating this in 'hexbin' and at some point getting this change onto CRAN?
We are trying to eliminate as many CRAN problems as possible before we commit to R-devel.

The PR also includes a suggestion to use upViewport() rather than popViewport(), so that it is possible to navigate back to viewports that 'hexbin' has created.

Paul

@edzer edzer merged commit 981b2fb into edzer:master Nov 11, 2019
@edzer
Copy link
Owner

edzer commented Nov 11, 2019

Thanks! When are you planning to commit to R-devel?

@pmur002
Copy link
Contributor Author

pmur002 commented Nov 11, 2019

There are a few more packages that we need to fix, so I think maybe only next year. The aim with this suggested fix is to take the timing out of it (the package should work both before and after we commit, without further intervention), so that we can hopefully chip away at the problems and then commit when ready. That's the aim anyway :)

When do you think a new 'hexbin' might make it to CRAN ?

@edzer
Copy link
Owner

edzer commented Nov 11, 2019

Just submitted it!

@edzer
Copy link
Owner

edzer commented Nov 12, 2019

On CRAN now.

@pmur002
Copy link
Contributor Author

pmur002 commented Nov 12, 2019

Awesome. Thanks very much!

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.

2 participants