[WIP] Support for alternative hashes (#1676)

[shcheklein]

• [x ] Have you followed the guidelines in our
  Contributing document?

• Does your PR affect documented changes or does it add new functionality
  that should be documented? If yes, have you created a PR for
  dvc.org documenting it or at
  least opened an issue for it? If so, please add a link to it.

---

Fixes #1676

Based on a fork by @vyloy .

[shcheklein]

#1920 - address comments from the previous PR

[Suor]

Why not simply:

return bool(modchecksum.checksum_types_from_str(...))

?

[Suor]

Could be:

return next((self.info[t] for t in self.remote.checksum_types() if t in self.info), None)
# or
return next(filter(bool, map(self.info.get, self.remote.checksum_types())), None) 

:)

[Suor]

Or

return self.info[self.checksum]

[Suor]

Make this properties as above?

[Suor]

Also is that an error if we have more than one here? If yes than I suggest adding an assert.

[Suor]

Use a decorator to abstract away caching?

[Suor]

No need to call .keys() just types = set(checksum_info). Actually Python has optimized code path to make sets from dicts. intersection is computed twice.

[Suor]

Is this addition_check only purpose to pass there self.changed_cache? If so won't it be more clear to simply use bool parameter?

[Suor]

Shouldn't this go to appropriate modules and just stay contained there? This will eliminate one level of indirection - these constants.

[shcheklein]

They will go to the appropriate modules for sure 👍

[Suor]

Looks like file_md5/file_checksum second result is never used. Maybe now is a good time to drop it and return only hex.

[Suor]

This is just:

SCHEMA.update({
    Optional(PARAM_CMD): Or(str, None),
    ...
})

[Suor]

Using ors instead of any() here will simplify code and provide short-circuit behavior.

[Suor]

checksum = {k: d[k] for k in modchecksum.CHECKSUM_MAP if d.get(k)}

# Or if you don't expect falsy values in `d`:
checksum = {k: d[k] for k in modchecksum.CHECKSUM_MAP if k in d}

[Suor]

Or using funcy:

from funcy import project
checksum = project(d, modchecksum.CHECKSUM_MAP)

Funcy is my lib :)

[Suor]

r = {k: v for k, v in self.checksum.items() if v} 

Or using funcy:

from funcy import compact
r = compact(self.checksum)

[Suor]

The whole thing is merging of two dicts with a filter:

from  itertools import chain

d1 = self.checksum
d2 = {...}
return {k: v for k, v in chain(d1.items(), d2.items()) if v}

# or with funcy
from funcy import merge, compact

d1 = self.checksum
d2 = {...}
return compact(merge(d1, d2))

[Suor]

Could be

if not set(checksum_types) <= set(supported_types):
    return None

[Suor]

A good place to use list comprehension.

[Suor]

These look like functional tests not unit ones. Also could be easily written in new pytest style using dvc fixture:

def test_compatibility(dvc):
    ret = main(...)
    assert ret == 0

    # ....

[shcheklein]

@Suor thanks for reviewing this. It was started by an external contributor and am trying to get it done, I don't expect it to be fast though. I'll ping you again when it's in a better shape and some fundamental issues are solved. I'll address your comments along the way, thanks!