search

eddelbuettel / digest

R package to create compact hash digests of R objects
https://eddelbuettel.github.io/digest
111 stars 47 forks source link

expand hashing algorithms that support `raw = TRUE` option #210

Closed pearsonca closed 2 months ago

pearsonca commented 2 months ago

I would like to use digest in some other work, but can't at the moment because the hashing algorithm I'd like to use doesn't support returning raw results.

I've worked up some changes to expand support for raw = TRUE for all the algorithms. I've added a test to confirm this is working.

eddelbuettel commented 2 months ago

Just got in from traveling, and will review. Thanks for taking the time to propose an improvement.

pearsonca commented 2 months ago

Of course - no rush. In the process of adding additional raw outputs, I tried to use a generic approach for the wrap up phase within the cases, and ended up applying that more generic approach to all of the hashers.

I moved the endian bits to the top in anticipation of needing them, but I didn't really have a handy way to check if the changes work under different endian-ness platforms. I suspect something can be done with the generic approach macros to make addressing that (outside of the hashing algorithms themselves) that will make handling endian-ness switch-like.

eddelbuettel commented 2 months ago

Yes. We can possibly split this also into more generic 'redoing of things' and a new 'raw export' feature.

I think none of the CRAN machines are of the other endian type now that the Solaris machine is retired. But we get this when I upload to Debian where I maintain this as a package too.

Also, you may have seen that @kevinushey caught last week that we needed a microtweak for 'STRICTHEADERS' for one of the imported hasher -- something I missed / ignored as it was third-party code, I use Rf prefixed whereever possibly for many years in my code -- and I just got the CRAN nag about it so I shipped a new 0.6.37 for which I will commit the small remaining changes once it is accepted. So we probably want a rebase anyway in due course. I'll ping you again.

pearsonca commented 2 months ago

Whitespace dressed, updated ChangeLog and DESCRIPTION, pruned MCP macro.

Standing by for some issue discussion, which I'm expecting to indicate swapping macro approach for proper functions (among other refactoring-towards-DRYer-implementation).

eddelbuettel commented 2 months ago

Thanks for all the extended changes, and of course the PR. Had to do a minor cleanup at end because I had committed from issue #211 in the meantime (more or less absent-mindedly).