Codecov Report

Merging #63 (2f52623) into master (779cea1) will increase coverage by 0.00%. The diff coverage is 100.00%.

@@           Coverage Diff           @@
##           master      #63   +/-   ##
=======================================
  Coverage   98.95%   98.95%           
=======================================
  Files          30       30           
  Lines         956      957    +1     
=======================================
+ Hits          946      947    +1     
  Misses         10       10

Flag	Coverage Δ
unittests	`98.95% <100.00%> (+<0.01%)`	:arrow_up:

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files	Coverage Δ
cytominer_eval/evaluate.py	`100.00% <ø> (ø)`
cytominer_eval/operations/precision_recall.py	`100.00% <100.00%> (ø)`
cytominer_eval/tests/test_evaluate.py	`100.00% <100.00%> (ø)`
...val/tests/test_operations/test_precision_recall.py	`100.00% <100.00%> (ø)`

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 779cea1...2f52623. Read the comment docs.

michaelbornholdt commented 2 years ago

@gwaygenomics Ready for your review :)

gwaybio commented 2 years ago

@michaelbornholdt - can you resolve the merge conflict?

Also, I feel like @Flohu is better equipped to review this PR than me. @FloHu, given your insights in #62, do you have availability to review within Michael's requested time frame?

michaelbornholdt commented 2 years ago

@FloHu. Everything is ready now.

FloHu commented 2 years ago

Sorry for my late answer, things have been very busy. Sure, I'll review it, give me time until tomorrow, it's on my list. :)

michaelbornholdt commented 2 years ago

@gwaygenomics please Merge :)

michaelbornholdt commented 2 years ago

@FloHu

Everything seems to be correct and making the groupby explicit is definitely needed. I have to say that I still do not see how it solves the issue about counting twice vs. counting once (as discussed in the issue, see last comment) but I trust you on it. When I have some more time I can think into it further.

If you have unique groupings now, ie one group per sample. Then there will no bidirectional vertices since you are only looking at one node. If you group by several nodes, then yes we still have the counting twice thing

cytomining / cytominer-eval

Quick fix to precision recall #63

Codecov Report