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

Use libsodium and scrypt for hash_names #7

Merged
merged 3 commits into from
Oct 3, 2018

Conversation

dirkschumacher
Copy link
Member

This uses libsodium's scrypt implementation. It also returns the data frame with stringsAsFactors=FALSE. Plus I changed the doc slightly to emphasize that a secret, random salt should be used. Oh and the salt needs to be length=1 (i.e. one salt for all inputs).

However the interface stays the same and I tried to leave the changes to a minimum.

Addresses #4 and #6

Downside/Upside: scrypt is about 40 times slower than sha1

# with plain sha1
microbenchmark::microbenchmark(epitrix::hash_names("watwat"))
#> Unit: microseconds
#>                           expr     min      lq    mean   median       uq
#>  epitrix::hash_names("watwat") 726.322 821.655 1242.65 995.4295 1098.189
#>       max neval
#>  23509.71   100

# with scrypt
microbenchmark::microbenchmark(epitrix::hash_names("watwat"))
#> Unit: milliseconds
#>                           expr      min       lq     mean   median
#>  epitrix::hash_names("watwat") 37.51021 38.46791 39.91234 39.19013
#>        uq      max neval
#>  40.33603 67.45346   100

@codecov
Copy link

codecov bot commented Sep 28, 2018

Codecov Report

Merging #7 into master will not change coverage.
The diff coverage is 100%.

Impacted file tree graph

@@          Coverage Diff          @@
##           master     #7   +/-   ##
=====================================
  Coverage     100%   100%           
=====================================
  Files           5      5           
  Lines          93    101    +8     
=====================================
+ Hits           93    101    +8
Impacted Files Coverage Δ
R/hash_names.R 100% <100%> (ø) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update d9c369b...0082c0a. Read the comment docs.

@thibautjombart
Copy link
Contributor

thibautjombart commented Oct 1, 2018 via email

@thibautjombart thibautjombart merged commit f3bd9c9 into reconhub:master Oct 3, 2018
@thibautjombart
Copy link
Contributor

@dirkschumacher can you also add yourself to the authors in the DESCRIPTION file?

@dirkschumacher dirkschumacher deleted the newhash branch October 3, 2018 16:08
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

Successfully merging this pull request may close these issues.

2 participants