mehta-lab / multiSero

serological measurements from multiplexed ELISA assays
BSD 3-Clause "New" or "Revised" License
6 stars 3 forks source link

when building a target image, must slice into it in [col, row] order #66

Closed bryantChhun closed 4 years ago

bryantChhun commented 4 years ago

very small fix in the way the composite_spots_img is generated.

The target image needs to be sliced in [col, row] order. This only influences the way debug data is generated.

codecov-commenter commented 4 years ago

Codecov Report

Merging #66 into master will not change coverage. The diff coverage is 0.00%.

Impacted file tree graph

@@           Coverage Diff           @@
##           master      #66   +/-   ##
=======================================
  Coverage   35.80%   35.80%           
=======================================
  Files          23       23           
  Lines        1606     1606           
=======================================
  Hits          575      575           
  Misses       1031     1031           
Flag Coverage Δ
#unittests 35.80% <0.00%> (ø)
Impacted Files Coverage Δ
array_analyzer/load/debug_plots.py 14.43% <0.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 4dbef22...b5c716f. Read the comment docs.

smguo commented 4 years ago

Just want to make sure I understand this PR... Here is the definition of regionprop bbox from scikit-image: """ bbox Bounding box (min_row, min_col, max_row, max_col). Pixels belonging to the bounding box are in the half-open interval [min_row; max_row) and [min_col; max_col). """

So the current indexing seems fine to me as it is... unless there is a flip of the order of the elements in bbox somewhere after it was generated?

bryantChhun commented 4 years ago

Let’s hold off on merging this PR because Jenny made a lot of changes with indexing in the uint16 PR. I think those fixed this problem but haven’t tested.

bryantChhun commented 4 years ago

@smguo @jennyfolkesson I tested this problem on the new master and it is fixed. Going to close this PR without merging.

If there's another discrepancy (say, master has somehow changed "find_spot_intensity" in a way that is not correct) let's make that a new PR.