What is the nature of your change?

[X] Enhancement (adds functionality).

Checklist

Please ensure that all boxes are checked before indicating that a pull request is ready for review.

[x] I have read the CONTRIBUTING.md guidelines.
[x] My code follows the style guidelines of this project.
[x] I have performed a self-review of my own code.
[x] I have commented my code, particularly in hard-to-understand areas.
[x] I have made corresponding changes to the documentation.
[x] My changes generate no new warnings.
[x] New and existing unit tests pass locally with my changes.
[ ] I have added tests that prove my fix is effective or that my feature works.
[ ] I have deleted all non-relevant text in this pull request template.

codecov-commenter commented 3 years ago

Codecov Report

Merging #50 (d27ecfa) into master (59684cf) will increase coverage by 0.10%. The diff coverage is 100.00%.

:exclamation: Current head d27ecfa differs from pull request most recent head 4c4c99b. Consider uploading reports for the commit 4c4c99b to get more accurate results

@@            Coverage Diff             @@
##           master      #50      +/-   ##
==========================================
+ Coverage   98.25%   98.36%   +0.10%     
==========================================
  Files          22       24       +2     
  Lines         802      855      +53     
==========================================
+ Hits          788      841      +53     
  Misses         14       14

Flag	Coverage Δ
unittests	`98.36% <100.00%> (+0.10%)`	:arrow_up:

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

Impacted Files	Coverage Δ
cytominer_eval/tests/test_transform/test_util.py	`100.00% <ø> (ø)`
cytominer_eval/evaluate.py	`100.00% <100.00%> (ø)`
cytominer_eval/operations/__init__.py	`100.00% <100.00%> (ø)`
cytominer_eval/operations/enrichment.py	`100.00% <100.00%> (ø)`
...iner_eval/tests/test_operations/test_enrichment.py	`100.00% <100.00%> (ø)`
cytominer_eval/transform/util.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 59684cf...4c4c99b. Read the comment docs.

gwaybio commented 3 years ago

nice! Looking good. What would you like me to look at? A quick first pass makes me think you still need to add some tests. See https://github.com/cytomining/cytominer-eval/tree/master/cytominer_eval/tests/test_operations for some examples

michaelbornholdt commented 3 years ago

Hey @gwaygenomics, All the checks are passing - that's good :)

michaelbornholdt commented 3 years ago

Whats left to do?

gwaybio commented 3 years ago

Now i'll review! (this is less daunting than it may sound) :)

michaelbornholdt commented 3 years ago

Btw, I cant set assignees and co right- because I am missing the right?

gwaybio commented 3 years ago

I think so. you did right to poke with a direct mention, i can self-assign

gwaybio commented 3 years ago

@michaelbornholdt - ready for you to take a look and address the PR comments

michaelbornholdt commented 3 years ago

Do the checks start running when all issues are resolved?

michaelbornholdt commented 3 years ago

@gwaygenomics Ready for review again. Notebook is currently running

michaelbornholdt commented 3 years ago

Yea thats correct @gwaygenomics

cytomining / cytominer-eval

Add enrichment operation #50

What is the nature of your change?

Checklist

Codecov Report