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

Unexpected behavior when printing the class instance object #37

Closed
oscardelama opened this issue Jan 5, 2015 · 11 comments

Comments

@oscardelama
Copy link
Contributor

commented Jan 5, 2015

Hi Winston.

I have a R6 class with an active binding named cov.df, which -of course- triggers some logic when that variable is read. For example, when this variable is read "too early" with respect to the expected usage of the class, the variable is empty, and its reading causes a friendly warning, giving to the user a hint about why the variable is empty. Something like this:

cov.df = function(value) {
  #  <some previous code>
  if (nrow(private$.cov.df) == 0)
    warning('There is no "cov" information. You should probably run the digest() function before.')
  return(private$.cov.df)
  # <some following code>
}

So far, so good. However, when I "print" the class instance object I get that friendly warning as if the variable has been read, but if I am just printing the class components, such reading is neither required nor expected:

> vo <- vvm$new()
> vo
#> <vvm>
#>   Public:
#>     <other class members>
#>     cov.df: active binding
#>     <more class members>
#>
#> Warning message:
#> In (function (value)  :
#>   There is no "cov" information. You should probably run the digest() function before.

In this case, the triggered behavior when printing the instance is harmless and I am commenting out those warnings while looking for a more suitable solution. Nevertheless, I am afraid those variable readings might have a negative -unexpected- impact, changing the state of the class, while it is trying to produce the requested (read) value.

Is there a way to circumvent these readings during the printing? i.e. Is there any flag letting me know the reading is made during a printing?

@gaborcsardi

This comment has been minimized.

Copy link
Member

commented Jan 5, 2015

I think you can define your own print method as a workaround.

@oscardelama

This comment has been minimized.

Copy link
Contributor Author

commented Jan 5, 2015

I am using R6 classes because, among other reasons, I don't know the S3 world. As a temporary way to circumvent this issue I may do that, but in the long term your suggestion is not a good idea.

Please, can you show me some code template to implement that print method and to do the intro-inspection to find out the class members?

@gaborcsardi

This comment has been minimized.

Copy link
Member

commented Jan 5, 2015

You don't need to know or do any S3. Just define an R6 class method called print. E.g. as in the manual: http://rstudio-pubs-static.s3.amazonaws.com/24456_042da2fcea0a405a9f07845a872ae7a6.html#printing-r6-objects-to-the-screen

@wch

This comment has been minimized.

Copy link
Member

commented Jan 5, 2015

I think @gaborcsardi's solution is the way to go. The print.R6 method will try to print out everything, and if you want different behavior you'll have to implement your own print method.

The definition of that function is here:
https://github.com/wch/R6/blob/618a64302e62b2f6ade8035d085e7c935311f27c/R/print.R#L2-L25

Note that this function calls the indent and object_summaries functions (also in that file) .

@gaborcsardi

This comment has been minimized.

Copy link
Member

commented Jan 5, 2015

Btw. it might be worth adding something about this to the docs, because it can be surprising.

@wch

This comment has been minimized.

Copy link
Member

commented Jan 5, 2015

Agreed. Kind of busy with other projects at the moment, but I'll consider pull requests. :)

@oscardelama

This comment has been minimized.

Copy link
Contributor Author

commented Jan 5, 2015

@wch: You say:

The print.R6 method will try to print out everything, and if you want different behavior
you'll have to implement your own print method.

I think you haven't understood entirely the issue. It is OK if the print.R6 method "prints everything". That is not the issue. The issue is that in the process, the print.R6 method reads the content of the active binding variables, which is really not required, because it (the print.R6 method) only prints their names, not their contents.

An active binding may be linked to a variable representing a "synthetic" value, which is built by request: when the variable (active binding) is read. So, the reading of those variables during the printing process, unexpectedly triggers the execution of code with different effects, from warning or error messages to changing the state of the instance object. This is not a trivial issue.

I think that fixing this issue for my classes might have, for me, almost the same cost than fixing it in the original print.r6 method. I will take a look to see if I can propose a pull request.

@gaborcsardi

This comment has been minimized.

Copy link
Member

commented Jan 5, 2015

Oh, I see, sorry, my bad.

I guess you need to rewrite this part: https://github.com/wch/R6/blob/618a64302e62b2f6ade8035d085e7c935311f27c/R/print.R#L73
to only extract x[[name]] into obj if it is not an active binding. You just need to reorganize a couple of lines.

@oscardelama

This comment has been minimized.

Copy link
Contributor Author

commented Jan 5, 2015

It's done.
The change was minimal. Basically it is just the change of sequence of two statements.
I have also added few lines of code to the test "Active bindings work" in test-portable.r to include this case and make sure this issue stays fixed. ;-)

@wch

This comment has been minimized.

Copy link
Member

commented Jan 6, 2015

Ah, I see now. Thanks for submitting the PR.

@wch

This comment has been minimized.

Copy link
Member

commented Jan 6, 2015

Fixed by #38.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
3 participants
You can’t perform that action at this time.