Skip to content

Conversation

@tkaemming
Copy link
Contributor

This splits much of the tests into a MinHashIndexBackendTestMixin which can be used to test any index backend, regardless of how it is implemented. Each backend will need it's own import/export test since the implementations are backend-specific.

Copy link
Contributor Author

@tkaemming tkaemming left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Each backend will need it's own import/export test since the implementations are backend-specific.

This is not 100% true, there probably could be a better way to test this


return MetricsWrapper(
RedisMinHashIndexBackend(
RedisScriptMinHashIndexBackend(
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Probably didn't need to make this change yet

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

My plan is to add another backend that doesn't depend on Lua scripting and use the base tests to ensure parity between the existing implementation and the new one

@ghost
Copy link

ghost commented Oct 13, 2017

1 Warning
⚠️ You should update CHANGES due to the size of this PR

Generated by 🚫 danger

Copy link
Member

@evanpurkhiser evanpurkhiser left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

clean, I like it.

@tkaemming tkaemming merged commit 4dee22d into master Oct 16, 2017
@tkaemming tkaemming deleted the similarity-python branch October 16, 2017 18:24
@github-actions github-actions bot locked and limited conversation to collaborators Dec 22, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants