Repository: spacetx/starfish

Issue:
decoding method in seqFISH
after generation of intensityTable by LocalSearchBlobDetecor I use codebook.decode_per_round_max(intensities) to decode each spot. It does not work(error: all-Nan slice encountered).
<img width="923" alt="Screenshot 2019-08-06 at 19 43 20" src="https://user-images.githubusercontent.com/50377927/62563690-d24d0400-b883-11e9-9582-5d20d8d2449e.png">
                                                  

Discussion:
codecov-io: # [Codecov](https://codecov.io/gh/spacetx/starfish/pull/1602?src=pr&el=h1) Report
> Merging [#1602](https://codecov.io/gh/spacetx/starfish/pull/1602?src=pr&el=desc) into [tonytung-fix-dims-alltest](https://codecov.io/gh/spacetx/starfish/commit/005b513b753f36540021331ae7ccf1fc78ff9448?src=pr&el=desc) will **decrease** coverage by `31.94%`.
> The diff coverage is `100%`.

[![Impacted file tree graph](https://codecov.io/gh/spacetx/starfish/pull/1602/graphs/tree.svg?width=650&token=kliQHejt6T&height=150&src=pr)](https://codecov.io/gh/spacetx/starfish/pull/1602?src=pr&el=tree)

```diff
@@                      Coverage Diff                      @@
##           tonytung-fix-dims-alltest   #1602       +/-   ##
=============================================================
- Coverage                      87.54%   55.6%   -31.95%     
=============================================================
  Files                            145     145               
  Lines                           5036    5041        +5     
=============================================================
- Hits                            4409    2803     -1606     
- Misses                           627    2238     +1611
```


| [Impacted Files](https://codecov.io/gh/spacetx/starfish/pull/1602?src=pr&el=tree) | Coverage Δ | |
|---|---|---|
| [starfish/core/codebook/codebook.py](https://codecov.io/gh/spacetx/starfish/pull/1602/diff?src=pr&el=tree#diff-c3RhcmZpc2gvY29yZS9jb2RlYm9vay9jb2RlYm9vay5weQ==) | `83.11% <100%> (-13.53%)` | :arrow_down: |
| [starfish/test/full\_pipelines/api/test\_dartfish.py](https://codecov.io/gh/spacetx/starfish/pull/1602/diff?src=pr&el=tree#diff-c3RhcmZpc2gvdGVzdC9mdWxsX3BpcGVsaW5lcy9hcGkvdGVzdF9kYXJ0ZmlzaC5weQ==) | `19.67% <0%> (-80.33%)` | :arrow_down: |
| [...e/experiment/builder/test/factories/all\_purpose.py](https://codecov.io/gh/spacetx/starfish/pull/1602/diff?src=pr&el=tree#diff-c3RhcmZpc2gvY29yZS9leHBlcmltZW50L2J1aWxkZXIvdGVzdC9mYWN0b3JpZXMvYWxsX3B1cnBvc2UucHk=) | `21.27% <0%> (-78.73%)` | :arrow_down: |
| [starfish/test/full\_pipelines/api/test\_iss\_api.py](https://codecov.io/gh/spacetx/starfish/pull/1602/diff?src=pr&el=tree#diff-c3RhcmZpc2gvdGVzdC9mdWxsX3BpcGVsaW5lcy9hcGkvdGVzdF9pc3NfYXBpLnB5) | `23.07% <0%> (-76.93%)` | :arrow_down: |
| [starfish/test/full\_pipelines/api/test\_merfish.py](https://codecov.io/gh/spacetx/starfish/pull/1602/diff?src=pr&el=tree#diff-c3RhcmZpc2gvdGVzdC9mdWxsX3BpcGVsaW5lcy9hcGkvdGVzdF9tZXJmaXNoLnB5) | `24.39% <0%> (-75.61%)` | :arrow_down: |
| [...ilder/test/structured\_formatter/test\_single\_fov.py](https://codecov.io/gh/spacetx/starfish/pull/1602/diff?src=pr&el=tree#diff-c3RhcmZpc2gvY29yZS9leHBlcmltZW50L2J1aWxkZXIvdGVzdC9zdHJ1Y3R1cmVkX2Zvcm1hdHRlci90ZXN0X3NpbmdsZV9mb3YucHk=) | `24.63% <0%> (-75.37%)` | :arrow_down: |
| [...re/spots/DetectSpots/local\_search\_blob\_detector.py](https://codecov.io/gh/spacetx/starfish/pull/1602/diff?src=pr&el=tree#diff-c3RhcmZpc2gvY29yZS9zcG90cy9EZXRlY3RTcG90cy9sb2NhbF9zZWFyY2hfYmxvYl9kZXRlY3Rvci5weQ==) | `25% <0%> (-70.2%)` | :arrow_down: |
| [...re/spots/DetectPixels/combine\_adjacent\_features.py](https://codecov.io/gh/spacetx/starfish/pull/1602/diff?src=pr&el=tree#diff-c3RhcmZpc2gvY29yZS9zcG90cy9EZXRlY3RQaXhlbHMvY29tYmluZV9hZGphY2VudF9mZWF0dXJlcy5weQ==) | `28.44% <0%> (-69.73%)` | :arrow_down: |
| [starfish/core/spacetx\_format/test\_fuzz.py](https://codecov.io/gh/spacetx/starfish/pull/1602/diff?src=pr&el=tree#diff-c3RhcmZpc2gvY29yZS9zcGFjZXR4X2Zvcm1hdC90ZXN0X2Z1enoucHk=) | `31.03% <0%> (-68.97%)` | :arrow_down: |
| [starfish/core/experiment/builder/\_\_init\_\_.py](https://codecov.io/gh/spacetx/starfish/pull/1602/diff?src=pr&el=tree#diff-c3RhcmZpc2gvY29yZS9leHBlcmltZW50L2J1aWxkZXIvX19pbml0X18ucHk=) | `29.21% <0%> (-67.42%)` | :arrow_down: |
| ... and [76 more](https://codecov.io/gh/spacetx/starfish/pull/1602/diff?src=pr&el=tree-more) | |

------

[Continue to review full report at Codecov](https://codecov.io/gh/spacetx/starfish/pull/1602?src=pr&el=continue).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/spacetx/starfish/pull/1602?src=pr&el=footer). Last update [005b513...a29ddc6](https://codecov.io/gh/spacetx/starfish/pull/1602?src=pr&el=lastupdated). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments).


Patch:
diff --git a/notebooks/ISS.ipynb b/notebooks/ISS.ipynb
index d6bf6dbc..ad5fe1b5 100644
--- a/notebooks/ISS.ipynb
+++ b/notebooks/ISS.ipynb
@@ -450,4 +450,4 @@
  },
  "nbformat": 4,
  "nbformat_minor": 2
-}
\ No newline at end of file
+}
diff --git a/starfish/core/codebook/codebook.py b/starfish/core/codebook/codebook.py
index 62a912bb..c531625d 100644
--- a/starfish/core/codebook/codebook.py
+++ b/starfish/core/codebook/codebook.py
@@ -651,7 +651,16 @@ class Codebook(xr.DataArray):
                 distances=(Features.AXIS, np.empty(0, dtype=np.float64)),
                 passes_threshold=(Features.AXIS, np.empty(0, dtype=bool)))
 
-        max_channels = intensities.argmax(Axes.CH.value)
+        intensities_without_nans = intensities.fillna(0)
+        max_channels = intensities_without_nans.argmax(Axes.CH.value)
+        # this snippet of code finds all the (feature, round) spots that have uniform illumination,
+        # and assigns them to a ch number that's one larger than max possible to ensure that such
+        # spots decode to `NaN`.
+        max_channels_max = intensities_without_nans.reduce(np.amax, Axes.CH.value)
+        max_channels_min = intensities_without_nans.reduce(np.amin, Axes.CH.value)
+        uniform_illumination_mask = (max_channels_max == max_channels_min).values
+
+        max_channels.values[uniform_illumination_mask] = intensities.sizes[Axes.CH.value]
         codes = self.argmax(Axes.CH.value)
 
         # TODO ambrosejcarr, dganguli: explore this quality score further


Test patch:
diff --git a/starfish/core/codebook/test/test_per_round_max_decode.py b/starfish/core/codebook/test/test_per_round_max_decode.py
index d7cd5a07..29077cec 100644
--- a/starfish/core/codebook/test/test_per_round_max_decode.py
+++ b/starfish/core/codebook/test/test_per_round_max_decode.py
@@ -155,3 +155,42 @@ def test_argmax_selects_the_last_equal_intensity_channel_and_decodes_consistentl
 
     decoded_intensities = codebook.decode_per_round_max(intensities)
     assert np.array_equal(decoded_intensities[Features.TARGET].values, ['nan', 'GENE_A'])
+
+
+def test_argmax_does_not_select_first_code():
+    """
+    When all the channels in a round are uniform, argmax erroneously picks the first channel as the
+    max.  In this case, it incorrectly assigns the wrong code for that round.  This test ensures
+    that the workaround we put in for this works correctly.
+    """
+
+    data = np.array(
+        [[[0.0, 1.0],
+          [1.0, 1.0]],  # this round is uniform, so it will erroneously be decoded as the first ch.
+         [[0.0, 1.0],
+          [1.0, 0.0]]]
+    )
+    intensities = intensity_table_factory(data)
+    codebook = codebook_factory()
+
+    decoded_intensities = codebook.decode_per_round_max(intensities)
+    assert np.array_equal(decoded_intensities[Features.TARGET].values, ['nan', 'GENE_A'])
+
+
+def test_feature_round_all_nan():
+    """
+    When all the channels in a round are NaN, argmax chokes.  This test ensures that the workaround
+    we put in for this works correctly.
+    """
+
+    data = np.array(
+        [[[0.0, 1.0],
+          [np.nan, np.nan]],
+         [[0.0, 1.0],
+          [1.0, 0.0]]]
+    )
+    intensities = intensity_table_factory(data)
+    codebook = codebook_factory()
+
+    decoded_intensities = codebook.decode_per_round_max(intensities)
+    assert np.array_equal(decoded_intensities[Features.TARGET].values, ['nan', 'GENE_A'])
diff --git a/starfish/test/full_pipelines/api/test_iss_api.py b/starfish/test/full_pipelines/api/test_iss_api.py
index b197ce89..7fb2ec7f 100644
--- a/starfish/test/full_pipelines/api/test_iss_api.py
+++ b/starfish/test/full_pipelines/api/test_iss_api.py
@@ -105,7 +105,7 @@ def test_iss_pipeline_cropped_data(tmpdir):
     assert np.array_equal(genes, np.array(['ACTB', 'CD68', 'CTSL2', 'EPCAM',
                                            'ETV4', 'GAPDH', 'GUS', 'HER2', 'RAC1',
                                            'TFRC', 'TP53', 'VEGF']))
-    assert np.array_equal(gene_counts, [20, 1, 5, 2, 1, 11, 1, 3, 2, 1, 1, 2])
+    assert np.array_equal(gene_counts, [19, 1, 5, 2, 1, 11, 1, 3, 2, 1, 1, 2])
 
     masks = iss.masks