gwaybio commented 3 years ago

The utility functions were getting clunky. I went to add new functionality to grit, but I struggled to figure out exactly where I should be adding it. This led me to this refactoring exercise.

No functionality was changed in this pull request. Only names and locations of python utility functions and tests. I also needed to modify several additional functions to fix import statements.

I also made super minor changes to formatting and ~fixed a deprecated testing warning~ (reverted back, see #56 ).

Note, I did fix a separate deprecation warning for numpy built-in types (see https://numpy.org/doc/stable/release/1.20.0-notes.html#deprecations)

gwaybio commented 3 years ago

cc @michaelbornholdt - I just gave you triage access to the repo, so that you are able to more effectively review this PR. Once you accept the invite, I will add you as a reviewer. Thanks! :)

codecov-commenter commented 3 years ago

Codecov Report

Merging #55 (03c09ee) into master (7f94b11) will increase coverage by 0.06%. The diff coverage is 100.00%.

@@            Coverage Diff             @@
##           master      #55      +/-   ##
==========================================
+ Coverage   98.38%   98.45%   +0.06%     
==========================================
  Files          24       30       +6     
  Lines         867      905      +38     
==========================================
+ Hits          853      891      +38     
  Misses         14       14

Flag	Coverage Δ
unittests	`98.45% <100.00%> (+0.06%)`	:arrow_up:

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

Impacted Files	Coverage Δ
...iner_eval/tests/test_operations/test_enrichment.py	`100.00% <ø> (ø)`
cytominer_eval/evaluate.py	`100.00% <100.00%> (ø)`
cytominer_eval/operations/enrichment.py	`100.00% <100.00%> (ø)`
cytominer_eval/operations/grit.py	`100.00% <100.00%> (ø)`
cytominer_eval/operations/mp_value.py	`100.00% <100.00%> (ø)`
cytominer_eval/operations/precision_recall.py	`100.00% <100.00%> (ø)`
...miner_eval/operations/replicate_reproducibility.py	`100.00% <100.00%> (ø)`
cytominer_eval/tests/test_evaluate.py	`100.00% <100.00%> (ø)`
cytominer_eval/tests/test_operations/test_grit.py	`100.00% <100.00%> (ø)`
...ominer_eval/tests/test_operations/test_mp_value.py	`100.00% <100.00%> (ø)`
... and 19 more

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 7f94b11...03c09ee. Read the comment docs.

gwaybio commented 3 years ago

alright @michaelbornholdt - this is ready for your eyes.

This blog post might be helpful to determine how to review.

Personally, I find reviewing other PRs to be helpful for the times when it's me actually proposing the changes. For this reason (and for an independent reason: I think more eyes on code the better) I am requesting your review. Do you think you'll be able to do this in a timely fashion? Hopefully by tomorrow?

FWIW, I think this PR is an easy one to start with. I'm not changing any functionality, just structure.

michaelbornholdt commented 3 years ago

And Black checks out with no errors! NICE

michaelbornholdt commented 3 years ago

@gwaygenomics

gwaybio commented 3 years ago

Awesome, I will merge now. Thanks!

Ideally, you would also want to use pipenv instead of requirements.txt

I am interested. Can you elaborate? Perhaps you can elaborate in a new github issue so we can possibly implement this enhancement in the future?

cytomining / cytominer-eval

Refactor utility functions #55

Codecov Report