[MERGE] fix divide by zero issue in MinHash.contained_by #572

ctb · 2018-12-05T01:16:36Z

Is it mergeable?
make test Did it pass the tests?
make coverage Is the new code covered?
Did it change the command-line interface? Only additions are allowed
without a major version increment. Changing file formats also requires a
major version number increment.
Was a spellchecker run on the source code and documentation after
changes were made?

codecov · 2018-12-05T01:23:15Z

Codecov Report

Merging #572 into master will not change coverage.
The diff coverage is n/a.

@@           Coverage Diff           @@
##           master     #572   +/-   ##
=======================================
  Coverage   88.51%   88.51%           
=======================================
  Files          25       25           
  Lines        3543     3543           
  Branches       37       37           
=======================================
  Hits         3136     3136           
  Misses        407      407

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update d2e2e3f...93b8343. Read the comment docs.

luizirber · 2018-12-05T01:22:57Z

sourmash/_minhash.pyx

@@ -364,6 +364,8 @@ cdef class MinHash(object):
        """\
        Calculate how much of self is contained by other.
        """
+        if not self.get_mins():


Suggested change

if not self.get_mins():

if len(self) == 0:

will avoid building mins (len() check the underlying mins in C++ without having to build it)

ctb · 2018-12-05T01:33:18Z

done!

* a trial refactoring of the lca db * save and load seem to basically work * got search & gather on LCA working in new framework * allow key error * keep identities with no lineage assigment * add multigather * Hyperlink DOIs to preferred resolver (#562) * fix divide by zero issue in MinHash.contained_by (#572) * fix divide by zero issue in contained_by * remove unused lineages and identifiers * update report output * majority of lca tests passing now! * fix gather? * ...all tests pass?

Updates the requirements on [pytest-cov](https://github.com/pytest-dev/pytest-cov) to permit the latest version. <details> <summary>Changelog</summary> <p><em>Sourced from <a href="https://github.com/pytest-dev/pytest-cov/blob/master/CHANGELOG.rst">pytest-cov's changelog</a>.</em></p> <blockquote> <h2>5.0.0 (2024-03-24)</h2> <ul> <li>Removed support for xdist rsync (now deprecated). Contributed by Matthias Reichenbach in <code>[#623](pytest-dev/pytest-cov#623) <https://github.com/pytest-dev/pytest-cov/pull/623></code>_.</li> <li>Switched docs theme to Furo.</li> <li>Various legacy Python cleanup and CI improvements. Contributed by Christian Clauss and Hugo van Kemenade in <code>[#630](pytest-dev/pytest-cov#630) <https://github.com/pytest-dev/pytest-cov/pull/630></code><em>, <code>[#631](pytest-dev/pytest-cov#631) <https://github.com/pytest-dev/pytest-cov/pull/631></code></em>, <code>[#632](pytest-dev/pytest-cov#632) <https://github.com/pytest-dev/pytest-cov/pull/632></code>_ and <code>[#633](pytest-dev/pytest-cov#633) <https://github.com/pytest-dev/pytest-cov/pull/633></code>_.</li> <li>Added a <code>pyproject.toml</code> example in the docs. Contributed by Dawn James in <code>[#626](pytest-dev/pytest-cov#626) <https://github.com/pytest-dev/pytest-cov/pull/626></code>_.</li> <li>Modernized project's pre-commit hooks to use ruff. Initial POC contributed by Christian Clauss in <code>[#584](pytest-dev/pytest-cov#584) <https://github.com/pytest-dev/pytest-cov/pull/584></code>_.</li> </ul> <h2>4.1.0 (2023-05-24)</h2> <ul> <li>Updated CI with new Pythons and dependencies.</li> <li>Removed rsyncdir support. This makes pytest-cov compatible with xdist 3.0. Contributed by Sorin Sbarnea in <code>[#558](pytest-dev/pytest-cov#558) <https://github.com/pytest-dev/pytest-cov/pull/558></code>_.</li> <li>Optimized summary generation to not be performed if no reporting is active (for example, when <code>--cov-report=''</code> is used without <code>--cov-fail-under</code>). Contributed by Jonathan Stewmon in <code>[#589](pytest-dev/pytest-cov#589) <https://github.com/pytest-dev/pytest-cov/pull/589></code>_.</li> <li>Added support for JSON reporting. Contributed by Matthew Gamble in <code>[#582](pytest-dev/pytest-cov#582) <https://github.com/pytest-dev/pytest-cov/pull/582></code>_.</li> <li>Refactored code to use f-strings. Contributed by Mark Mayo in <code>[#572](pytest-dev/pytest-cov#572) <https://github.com/pytest-dev/pytest-cov/pull/572></code>_.</li> <li>Fixed a skip in the test suite for some old xdist. Contributed by a bunch of people in <code>[#565](pytest-dev/pytest-cov#565) <https://github.com/pytest-dev/pytest-cov/pull/565></code>_.</li> </ul> <h2>4.0.0 (2022-09-28)</h2> <p><strong>Note that this release drops support for multiprocessing.</strong></p> <ul> <li> <p><code>--cov-fail-under</code> no longer causes <code>pytest --collect-only</code> to fail Contributed by Zac Hatfield-Dodds in <code>[#511](pytest-dev/pytest-cov#511) <https://github.com/pytest-dev/pytest-cov/pull/511></code>_.</p> </li> <li> <p>Dropped support for multiprocessing (mostly because <code>issue 82408 <https://github.com/python/cpython/issues/82408></code>_). This feature was mostly working but very broken in certain scenarios and made the test suite very flaky and slow.</p> <p>There is builtin multiprocessing support in coverage and you can migrate to that. All you need is this in your <code>.coveragerc</code>::</p> <p>[run] concurrency = multiprocessing</p> </li> </ul>  </blockquote> <p>... (truncated)</p> </details> <details> <summary>Commits</summary> <ul> <li><a href="https://github.com/pytest-dev/pytest-cov/commit/5295ce01c84262cec88f31255e9ac538718f3047"><code>5295ce0</code></a> Bump version: 4.1.0 → 5.0.0</li> <li><a href="https://github.com/pytest-dev/pytest-cov/commit/1181b067972bf94569f8011f3b18f271690f9ab1"><code>1181b06</code></a> Update changelog.</li> <li><a href="https://github.com/pytest-dev/pytest-cov/commit/9757222e2e044361e70125ebdd96e5eb87395983"><code>9757222</code></a> Fix a minor grammar error (<a href="https://github.com/pytest-dev/pytest-cov/issues/636">#636</a>)</li> <li><a href="https://github.com/pytest-dev/pytest-cov/commit/9f5cd81a0dbe3fe41681efdbef516c08988fe8ff"><code>9f5cd81</code></a> Cleanup releasing instructions. Closes <a href="https://github.com/pytest-dev/pytest-cov/issues/616">#616</a>.</li> <li><a href="https://github.com/pytest-dev/pytest-cov/commit/93b5047ec5050d63c10a6fe16a09b671a7a03df8"><code>93b5047</code></a> Add test for pyproject.toml loading without explicit --cov-config. Ref <a href="https://github.com/pytest-dev/pytest-cov/issues/508">#508</a>.</li> <li><a href="https://github.com/pytest-dev/pytest-cov/commit/ff50860d7c67b920503745d92a3f0944cf41f982"><code>ff50860</code></a> docs: add config instructions for pyproject.toml.</li> <li><a href="https://github.com/pytest-dev/pytest-cov/commit/4a5a4b5fa4b1c63ddcab5cbc1813798c9b6f1d36"><code>4a5a4b5</code></a> Keep GitHub Actions up to date with GitHub's Dependabot</li> <li><a href="https://github.com/pytest-dev/pytest-cov/commit/1d7f55963d5138f41c452a946f7cca7e0b6ee8b2"><code>1d7f559</code></a> Fix or remove URLs that are causing docs tests to fail</li> <li><a href="https://github.com/pytest-dev/pytest-cov/commit/6a5af8e85b8242ac815f33e26adf9068f5f0ebc3"><code>6a5af8e</code></a> Update changelog.</li> <li><a href="https://github.com/pytest-dev/pytest-cov/commit/d9fe8dfed15023d3410dd299c5092e755b8981c2"><code>d9fe8df</code></a> Switch to furo. Closes <a href="https://github.com/pytest-dev/pytest-cov/issues/618">#618</a>.</li> <li>Additional commits viewable in <a href="https://github.com/pytest-dev/pytest-cov/compare/v4.0.0...v5.0.0">compare view</a></li> </ul> </details> <br /> Dependabot will resolve any conflicts with this PR as long as you don't alter it yourself. You can also trigger a rebase manually by commenting `@dependabot rebase`. [//]: # (dependabot-automerge-start) [//]: # (dependabot-automerge-end) --- <details> <summary>Dependabot commands and options</summary> <br /> You can trigger Dependabot actions by commenting on this PR: - `@dependabot rebase` will rebase this PR - `@dependabot recreate` will recreate this PR, overwriting any edits that have been made to it - `@dependabot merge` will merge this PR after your CI passes on it - `@dependabot squash and merge` will squash and merge this PR after your CI passes on it - `@dependabot cancel merge` will cancel a previously requested merge and block automerging - `@dependabot reopen` will reopen this PR if it is closed - `@dependabot close` will close this PR and stop Dependabot recreating it. You can achieve the same result by closing it manually - `@dependabot show <dependency name> ignore conditions` will show all of the ignore conditions of the specified dependency - `@dependabot ignore this major version` will close this PR and stop Dependabot creating any more for this major version (unless you reopen the PR or upgrade to it yourself) - `@dependabot ignore this minor version` will close this PR and stop Dependabot creating any more for this minor version (unless you reopen the PR or upgrade to it yourself) - `@dependabot ignore this dependency` will close this PR and stop Dependabot creating any more for this dependency (unless you reopen the PR or upgrade to it yourself) </details> Signed-off-by: dependabot[bot] <support@github.com> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>

fix divide by zero issue in contained_by

30870c0

luizirber approved these changes Dec 5, 2018

View reviewed changes

fix per luiz

93b8343

luizirber merged commit 4aab62f into master Dec 5, 2018

luizirber deleted the fix/divide_by_zero branch December 5, 2018 01:35

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[MERGE] fix divide by zero issue in MinHash.contained_by #572

[MERGE] fix divide by zero issue in MinHash.contained_by #572

ctb commented Dec 5, 2018 •

edited

Loading

codecov bot commented Dec 5, 2018 •

edited

Loading

luizirber Dec 5, 2018

ctb commented Dec 5, 2018

[MERGE] fix divide by zero issue in MinHash.contained_by #572

[MERGE] fix divide by zero issue in MinHash.contained_by #572

Conversation

ctb commented Dec 5, 2018 • edited Loading

codecov bot commented Dec 5, 2018 • edited Loading

Codecov Report

luizirber Dec 5, 2018

Choose a reason for hiding this comment

ctb commented Dec 5, 2018

ctb commented Dec 5, 2018 •

edited

Loading

codecov bot commented Dec 5, 2018 •

edited

Loading