`dput(<R7_object>)` errors

[t-kalinowski]

reprex:

library(S7)
dput(new_class("Foo")())
#> Error in dput(new_class("Foo")()): cannot get a slot ("slots") from an object of type "NULL"

^{Created on 2023-01-05 with reprex v2.0.2}

[hadley]

I suspect this is a consequence of:

library(R7)
foo <- new_class("Foo")()

isS4(foo)
#> [1] FALSE
typeof(foo)
#> [1] "S4"

^{Created on 2023-02-19 with reprex v2.0.2}

So might need adjustment in dput? (i.e. maybe this line is using the wrong criteria: https://github.com/wch/r-source/blob/8791c1706307083bc2f188a6b24fda1b5ef03cdf/src/main/deparse.c#L892)

[hadley]

See https://github.com/RConsortium/OOP-WG/issues/151#issuecomment-1032959174 for a little more context.

@t-kalinowski is this something you have time to look into?

[lawremi]

Perhaps @mmaechler could comment, since he is the author of that line. Perhaps just being extra careful to catch all cases, even though IS_S4_OBJECT() is sufficient?

[mmaechler]

o dear .. I have replied to this a few days ago, but must have lost it "in translation" ;-) That code change (r74131 | 2018-01-18) with log msg deparse() amends for S4(not typeof S4, no .Data) was part of an amendment to the large r74105 which finally introduced proper deparse/dput/... ing of S4 objects, notably those not* TYPEOF S4SXP .. but also for these IS_S4_OBJECT() is sufficient...

Some further Rchaeology has revealed that a previous version actually only used the equivalent of testing for S4SXP type. Now that is quite some evidence, that indeed the hasS4_t = TYPEOF(s) == S4SXP; if (IS_S4_OBJECT(s) || hasS4_t) {..} clause may have been mostly motivated by "back-compatible safety" and we could drop the || hasS4_t part there.

[t-kalinowski]

I pushed some commits testing how we might fix the error.

While dput(<S7_object>) no longer errors with the patches, the output, <S4 object>, is less than satisfying.

Taking a quick glance it at the rest of dput() sources, I’m guessing that we’ll need a separate code path for S7 similar to this for S4 (though there is still a very high probability that there could exists a 1-line elegant patch that redirects the S7 object to another code path where everything just works; I didn't see it but maybe I just didn't stare at the autostereogram long enough).

[t-kalinowski]

Just noticed https://github.com/r-devel/r-svn/commit/fb5d565a65d930a0fa3916f72f5903992b2d1b54 ! :tada:

[mmaechler]

Just noticed r-devel/r-svn@fb5d565 ! tada

... I wanted to mention it here ... but just then, it was not yet mirrored .. I'm sorry if you lost time unnecessarily!

[t-kalinowski]

Not at all, no worries :)

Note that https://github.com/r-devel/r-svn/commit/fb5d565a65d930a0fa3916f72f5903992b2d1b54 gets dput(<S7_object>) past the first hurdle, but still ends in an error when the S7 object falls through to here.

[t-kalinowski]

draft patch: https://github.com/r-devel/r-svn/pull/110/files

[t-kalinowski]

Patch submitted to bugzilla: https://bugs.r-project.org/show_bug.cgi?id=18511

[lawremi]

Separate from the error, it's worth discussing how to improve the output. Assuming we had a hook that tried to deparse an S7 object, what would be the most useful output? Since we aren't able to represent a class as a string, fully deparsing an object would be quite verbose. We could just assume that there is a class object assigned under the same name in the current scope, which in general is equivalent to S4 assuming that a class name is visible to new() in the current scope.

So instead of the S4

new("range", start = 1, end = 10)

we would output

range(start = 1, end = 10)

assuming that range refers to the class object.

[mmaechler]

Indeed; and with a non-error "backout" in the case there is no such a class object visible {in all of search() , probably?}

[hadley]

Resolved by patch to R.