Add update sha1.default() to work with any class

[billdenney]

Fix #224

[eddelbuettel]

Reverse-dependency checks came and went. (The machine I run these on is not often used for digest and specific per-package dependencies do not autoinstall so a fair number failed for 'dependency xyz missing' so I ran another baseline check with digest from the main branch. Came out the the same so "no change for worse".) While that is good I am still not a fan of the removal of the (arguably redundant) S3 accessors. How would you feel about reverting that part of the change?

[billdenney]

I reverted those removals.

[eddelbuettel]

Appreciate the reversal. I keep pondering this, and the actual hard error in there with stop() was part of the original support, and I think it makes sense --- it indicates that the S3 support is complete, just as it does in #224. So a more minimal PR may be my current preference.

I updated my branch, borrowed you additional tests and am running it in reverse dependency checks now.

[billdenney]

Is there a scenario where using sha1_attr_digest() for classes not specifically handled would be a problem? I ask because I discovered that <- and environment were missing when I tried to implement an sha1() method in the rxode2 library.

Since <- and environment are base R classes, it seemed better to put the methods here rather than into the rxode2 library. Enabling the sha1.default() function to handle any class by default via sha1_attr_digest() would prevent needing to update digest when new classes are necessary that don't fit within another library (as with <- and environment here).

Another example that would be fixed with a default method that provides a result is logLik (found just after I opened #224):

digest::sha1(logLik(lmx <- lm(x ~ 1, data = data.frame(x = 1:5))))
#> Error: sha1() has no method for the 'logLik' class

^{Created on 2025-01-03 with reprex v2.1.1}

My concern is that this becomes whack-a-mole when going through all the required and recommended libraries that are unlikely to provide their own sha1() S3 methods.

[eddelbuettel]

I think I prefer to follow the explicit S3 dispatch where we already use it that is in sha1. As demonstrated earlier, if sha1(foo) bites for whatever foo you have digest(foo, algo="sha1") should always be available

> library(digest)
> digest(logLik(lmx <- lm(x ~ 1, data = data.frame(x = 1:5))), algo="sha1")
[1] "e2f78853387414a8b41d7793383ab062aa27241e"
> 

meaning that if you need it you could (if push came to shove) define a local sha1 override for it.

The package is 20+ years old, the S3-based sha1 support is nine years old. I think on balance I rather stay with the design that worked than to (more radically than we need to) alter it now. The aim is to have something reliable for all/most use cases.

[eddelbuettel]

That said, whackamole is a risk, and logLik is a good example. There are a very large number of S3 classes out there, and we cannot possibly cover all. Then again digest() always works on "anything".

Maybe fusing our two competing PR and turning the stop() into a warning() (or message()) and proceeding is a good middle ground.

[eddelbuettel]

message() it is:

> library(digest)
> sha1(logLik(lmx <- lm(x ~ 1, data = data.frame(x = 1:5))))
sha1() has no method for the 'logLik' class, so using fallback.
[1] "7d7ba7a23dbe3ccc8342811bf6dce184501dc12a"
> suppressMessages(sha1(logLik(lmx <- lm(x ~ 1, data = data.frame(x = 1:5)))))
[1] "7d7ba7a23dbe3ccc8342811bf6dce184501dc12a"
> 

Committed in 7af4aa7. Thoughts?

[billdenney]

I like that compromise. I've merged the branches.

[eddelbuettel]

I hate hate hate hate this editor default of using four lines for what can be / should be two. Sadly I am in the minority on this.

No need to change though.

[eddelbuettel]

I tend to try to keep these things out of PRs but I can clean this up post-merge.

No need to change.

[eddelbuettel]

Nice. Some procrastination and noodling over alternatives made this better.

[eddelbuettel]

Should have altered the PR title (but then did so in an --amend commit). All good now.

Thanks for your attention to detail and patience on this.