Selector
Selector copied to clipboard
[Distance module] Add Formulas to Docstrings and Add Tests
Finalize the functions in distance module by:
- Unifying docstrings and adding formulas (that match the implementation)
- Add tests for each function (to cover all lines) and tricky cases
- Polish code to make sure it is readable (e.g., add comments) and pythonic
### Tasks
- [x] Move `sklearn_supported_metrics` from `utils` module to `distance` module
- [x] Move tests related to distance module into `test_distance.py`
- [x] Remove support for `sklearn.pairwise_distances` function as it only adds overhead to the code.
- [x] Add formulas
- [x] Add tests
### Tasks - Post PR#129
- [x] Test coverges: missing lines 184 and 189.
- [x] Fix math formula: https://github.com/theochem/DiverseSelector/blob/main/DiverseSelector/distance.py#L149
- [x] Having both `pairwise_similarity_bit` and `compute_distance_matrix` seems redundant considering the fact that we have the #123 module.
- [x] Remove empty lines at the end of the modules, like https://github.com/theochem/DiverseSelector/blob/main/DiverseSelector/test/test_distance.py#L99
- [x] Add tests to explicitly test `tanimoto` and `modified_tanimoto`
- [x] Implementation needs to be improved and further clarified: https://github.com/theochem/DiverseSelector/blob/main/DiverseSelector/distance.py#L223
- [x] Needs more clarification in docstring and comments of https://github.com/theochem/DiverseSelector/blob/main/DiverseSelector/distance.py#L143
I leave a note here, as this PR #137 is already merged. Before this issue can be closed:
- The
modified_tanimotostill needs to be finalized by @FarnazH. - The module name should be changed as it contains similarity measures, not distances.
- Shouldn't the
nearest_average_tanimotobe moved to thediversity.pymodule?
One more thing we need to have is adding explanations for MaxMin and OptiSim as discussed in #119.
Yes, this is a diversity measure. I agree that it should be moved.
- Shouldn't the
nearest_average_tanimotobe moved to thediversity.pymodule?