Skip to content

don't use hash_name but rather hash function directly#5

Open
prusnak wants to merge 1 commit intoTierion:masterfrom
prusnak:master
Open

don't use hash_name but rather hash function directly#5
prusnak wants to merge 1 commit intoTierion:masterfrom
prusnak:master

Conversation

@prusnak
Copy link
Copy Markdown

@prusnak prusnak commented Dec 28, 2017

Python idiom is to use duck typing, so let's use it. Also it is possible to use hash functions that your library don't recognize, for example pyblake2.blake2s or pyblake2.blake2b (which use the same API as functions from hashlib)

@EderSantana
Copy link
Copy Markdown
Collaborator

Hi @prusnak ,

Thanks for the PR.

Comment: I think we could support both modes. Quickly passing a string makes the API easier to use and remember, while passing a function makes it generalize better.
Do you want to check if the user passed a string or a function and handle both cases?
Also, your PR would be more complete with updates to the README docs.

(Optional)
Also, would you please add a conditional test with pyblake2? The condition tests if pyblake2 is installed and run the test. We could force the CI to install it

@prusnak
Copy link
Copy Markdown
Author

prusnak commented Jan 9, 2018

Unfortunately, I don't have time to implement proposed changes at the time, but they should be pretty straightforward. Maybe someone else will pick this up.

@EderSantana
Copy link
Copy Markdown
Collaborator

EderSantana commented Jan 9, 2018

Thanks! Let me leave this open then.
If either somebody else picks this up or we get more such requests, we work on it later.

@Aareon
Copy link
Copy Markdown

Aareon commented Apr 29, 2018

How about using it like this?

from merkletools import MerkleTools

class Merkle(MerkleTools):
    def __init__(self, hash_function):
        super().__init__()
        self.hash_function = hash_function

Now you have no constraint on the hash function, and so long as your hash function is hashlib styled, it will work with no problem

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.

3 participants