Skip to content

Update similarity.py to improve test coverage to 99%#295

Merged
FanwangM merged 5 commits intotheochem:mainfrom
mohini-aggarwal:update-similarity-94-coverage
Apr 14, 2026
Merged

Update similarity.py to improve test coverage to 99%#295
FanwangM merged 5 commits intotheochem:mainfrom
mohini-aggarwal:update-similarity-94-coverage

Conversation

@mohini-aggarwal
Copy link
Copy Markdown
Contributor

@mohini-aggarwal mohini-aggarwal commented Apr 3, 2026

  • Updated selector/measures/similarity.py to the 99% coverage version.
  • Added comprehensive tests in test_diversity_comprehensive.py.
  • Ready for review and integration into main repository.

@mohini-aggarwal
Copy link
Copy Markdown
Contributor Author

mohini-aggarwal commented Apr 3, 2026

Hi Mentors @FarnazH @FanwangM @marco-2023 !

I submitted this PR (#295 ) improving test coverage in similarity.py from 76% to 94%. This is part of my ongoing contributions to the Selector project for GSoC 2026.

Thank you for your guidance!

Copy link
Copy Markdown
Collaborator

@FanwangM FanwangM left a comment

Choose a reason for hiding this comment

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

Thanks for the PR. The current PR adds an empty line after each docstring/code block, which should be avoided. This also makes the code unreadable. Please fix this and we will do further code review. Thanks.

@mohini-aggarwal

@mohini-aggarwal mohini-aggarwal requested a review from FanwangM April 9, 2026 13:45
@mohini-aggarwal
Copy link
Copy Markdown
Contributor Author

Hi @FanwangM, I’ve updated the PR to address the formatting issues you pointed out. Please let me know if anything else needs improvement, happy to make further changes.
Thank You!

@FanwangM
Copy link
Copy Markdown
Collaborator

Hi @mohini-aggarwal . Thanks for the updates. I have checked your current PR and noticed that you only added/deleted empty lines, without any actual content changes. Also, some changes violate the coding style we follow, e.g., PEP8. For example, you left only one empty line between two functions, whereas we usually have two.

@mohini-aggarwal
Copy link
Copy Markdown
Contributor Author

Hi @FanwangM @FarnazH @marco-2023
I have refactored and improved the test suite for selector.measures.similarity.
Key improvement:

  • Coverage increased from 76% → 99% for similarity module
  • This was achieved by adding missing edge-case tests, improving validation coverage, and restructuring tests for better maintainability.
    Before vs After coverage screenshots are attached for reference.
Screenshot 2026-04-11 133508 Screenshot 2026-04-11 144233 Please let me know if any further changes are required. Thank you!

Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

Adds a new unit test module for selector.measures.similarity to increase coverage of the similarity utilities (pairwise similarity, scaling, and similarity indices).

Changes:

  • Introduces selector/measures/tests/test_similarity.py with tests for tanimoto, modified_tanimoto, pairwise_similarity_bit, scaled_similarity_matrix, and similarity_index.
  • Adds negative-path tests for invalid dimensions, shape mismatches, and unsupported metric/index names.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread selector/measures/tests/test_similarity.py
Comment thread selector/measures/tests/test_similarity.py
Comment thread selector/measures/tests/test_similarity.py Outdated
Comment thread selector/measures/tests/test_similarity.py
@mohini-aggarwal mohini-aggarwal changed the title Update similarity.py to improve test coverage to 94% Update similarity.py to improve test coverage to 99% Apr 12, 2026
@mohini-aggarwal
Copy link
Copy Markdown
Contributor Author

Hi @FanwangM,
Thanks for the feedback.

  • I’ve updated the test assertions to correctly handle NumPy scalar return types and synced the branch with the latest changes.
  • I’ve also ensured the formatting follows PEP8 and updated the PR description to reflect the actual changes.
    Please let me know if anything else needs improvement.
    Thanks!

Copy link
Copy Markdown
Collaborator

@FanwangM FanwangM left a comment

Choose a reason for hiding this comment

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

Looks great to me. Thanks.

@FanwangM FanwangM merged commit 72dfb1e into theochem:main Apr 14, 2026
17 checks passed
@mohini-aggarwal mohini-aggarwal deleted the update-similarity-94-coverage branch April 14, 2026 06:55
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