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

Issues masking S4 methods #15

Closed
daattali opened this issue Jun 29, 2015 · 12 comments
Closed

Issues masking S4 methods #15

daattali opened this issue Jun 29, 2015 · 12 comments

Comments

@daattali
Copy link
Owner

Reported by Jon Calder:

I've noticed that when attaching the shinyjs package, "removeClass", and "show" are masked from 'package:methods’. It took me a long time to work out that this was causing problems in the shiny app I am working on at the moment. This is because 'show' gets called when using print on an S4 object, which I happen to be using all over the place to check and debug the structure of S4 objects which I'm creating dynamically within the app.

Just to clarify with a simple example, if 'shinyjs' is attached while running the below code:

setClass("testS4Object",
         representation(
           ID = "numeric",
           Name = "character"
         ),
         prototype(
           ID = NA_real_,
           Name = NA_character_
         )
)

x = new("testS4Object")
show(x)

Then one gets the error:

Error in dynGetCopy("session") : ‘session’ not found

I assume the error originates here in this function: https://github.com/daattali/shinyjs/blob/master/R/jsFunc-aaa.R

Once I realized what the issue was, it was easy to get around this by using methods::show() instead of just show(), but since this error message only crops up when running the code in an interactive R session and not while running a shiny app (as will be the case with most people like me using your package for the first time), it took some time to work through a process of elimination and find the source of the problem.

Since S4 is widely used, I think it would probably be wise to rename your 'show' function to something else that won't cause this conflict for people going forward? Then it's one less thing to trip people up in the use of your package.

@daattali
Copy link
Owner Author

I can't think of a better name than show, I'm open to suggestions. I don't think I'll ever remove show because there are a lot of people already using it but maybe I'll add an alias. It seems weird to me though that printing an S4 object will call show instead of methods::show. Not sure if that's a "bug" with methods or is that for some reason the wanted behaviour

@daattali
Copy link
Owner Author

@jonmcalder
Copy link

Unfortunately I can't really offer more insight here. I ran into the problem quite unexpectedly and automatically assumed that if someone needed to budge it should be you since the methods package is maintained by the R core team. With that said I do appreciate your confusion with print calling show instead of methods::show.

@daattali
Copy link
Owner Author

Yes, I definitely agree that if anyone should budge, it should be me and not R :)
I spoke with Hadley and he also seemed to agree that this might be a bug with R, defining show should NOT be causing printing an object to break. I messaged the R-devel mailing list (as per his suggestion), I'll see what the response is.

@jonmcalder
Copy link

Cool - thanks!

@daattali
Copy link
Owner Author

This issue has been reported to the R core team and was fixed in R and will be available in R 3.3.0. Here is the commit that fixes this - SVN commit # 68702 (you can use the unstable R-devel version to see the fix live)

A shorter reproducible example of the bug (which is unrelated to shinyjs) is the following:

show <- function() {}
setClass("Person", slots = list(name = "character"))
tom <- new("Person", name = "Tom")
tom # error in R < 3.3.0
methods::show(tom) # works 

Just for completeness, I want to point out that in the original report it is said that calling show(x) results in an error, and that will still result in an error. BUT previously, simply calling x would also result in the same error because it was calling show(x) and was using shinyjs::show instead of methods::call, and that issue is now fixed - if you load shinyjs, define an S4 object, and try to print it by just typing its name - it will work.

Thanks for reporting @jonmcalder !

@Gnolam
Copy link

Gnolam commented Jul 24, 2015

Hi Dean,

Thank you very much for addressing this issue.
You have only forgotten to mention (or I did not find it) that you have already fixed it.
One merely needs to install the latest version of your package devtools::install_github("daattali/shinyjs") to fix it :)

Thank you, once again!

@daattali
Copy link
Owner Author

@Gnolam I did not fix it on my end :) In the dev version of R this won't happen, but as far as I know, if you're using R 3.2 you'll still see this problem. Can you show your sessionInfo() and tell me what shinyjs version you were using before updating?

@Gnolam
Copy link

Gnolam commented Jul 24, 2015

Hi Dean,

It would be difficult to recollect the shinyjs version exactly.
I have installed shinyjs from CRAN yesterday and it was showing bug. So it is the most recent official one.
After updating it with the most recent from github the bug has disappeared.

Please find the session info below:
R version 3.2.1 (2015-06-18)
Platform: x86_64-w64-mingw32/x64 (64-bit)
Running under: Windows 7 x64 (build 7601) Service Pack 1

locale:
[1] LC_COLLATE=English_Australia.1252 LC_CTYPE=English_Australia.1252
[3] LC_MONETARY=English_Australia.1252 LC_NUMERIC=C
[5] LC_TIME=English_Australia.1252

attached base packages:
[1] stats graphics grDevices utils datasets methods base

other attached packages:
[1] RPostgreSQL_0.4 DBI_0.3.1 shinyjs_0.0.8.2 shiny_0.12.1

loaded via a namespace (and not attached):
[1] R6_2.1.0 htmltools_0.2.6 tools_3.2.1 rstudioapi_0.3.1
[5] Rcpp_0.11.6 jsonlite_0.9.16 digest_0.6.8 xtable_1.7-4
[9] httpuv_1.3.2 mime_0.3

@daattali
Copy link
Owner Author

Thanks @Gnolam , that's still very strange. I just tried using R 3.2.1 and the latest shinyjs, and I do still get an error when running

library(shiny)
library(shinyjs)
setClass("testS4Object",
         representation(
           ID = "numeric",
           Name = "character"
         ),
         prototype(
           ID = NA_real_,
           Name = NA_character_
         )
)

x = new("testS4Object")
x

It's a different error than before because I changed the internal code, but it's still weird that on R < 3.3 you're not seeing an error with the above exact code.

@Gnolam
Copy link

Gnolam commented Jul 24, 2015

In this case it is strange indeed. As it is working.

I have recently reinstalled R/Rstudio from scratch.
May be there were other conflicts as well.

@Gnolam
Copy link

Gnolam commented Jul 28, 2015

Hi, I have tried once again and the results I get are:

An object of class "testS4Object"
Slot "ID":
[1] NA
Slot "Name":
[1] NA

Could be a problem caused by set of installed packages?

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

No branches or pull requests

3 participants