Add back channel normalization before preprocessing

[cliu72]

What is the purpose of this PR?

Add back channel normalization before pixel matrix generation, which seems to have been lost when parallelization was added. As in this commit: https://github.com/angelolab/ark-analysis/tree/370415cea71ec675c4be9f2c57a8644702390350 before the parallelization functionality was added, pixel values should be channel normalized before being fed into create_fov_pixel_data.

How did you implement your changes

Added back the code to channel normalize in preprocess_fov before calling create_fov_pixel_data.

Remaining issues

Don't think there are remaining issues.

[cliu72]

Looks like some test is failing after this change. @alex-l-kong can you take a look? Don't really know how to read these travis logs

[alex-l-kong]

@cliu72 thanks for looking into this, not sure why the channel normalization got deleted after merging master into the parallelization branch (better check my merge tool). I'll take a look at the tests.

[alex-l-kong]

@cliu72 the test needed the new argument for channel normalization values. Also, I updated preprocess_fov to take in channel_norm_df instead of channel_norm_path for consistency purposes (since we pass in pixel_norm_val and not pixel_norm_path).

[ngreenwald]

How did this slip past the tests? I thought we had a test that specifically checked that images were normalized correctly

[alex-l-kong]

@ngreenwald the test for calculate_channel_percentiles verifies that it works fine, however the test for create_pixel_matrix doesn't verify the actual values since it generates random values each time. I set it up this way to verify that the function is robust in subsetting to different values that the images could take on.

We could start using a pre-defined dataset to test, similar to how mibi-bin-tools is currently set up. Alternatively, we could consider further refactoring create_pixel_matrix and its helper functions. It's getting a bit bloated and separating out the different steps could make it easier to verify integrity. That way, we can more easily test specific components with hard-coded values, and then randomize the higher-level tests.

[ngreenwald]

This test I wrote checks to make sure the data has been divided by the right channel-specific normalization factor:
https://github.com/angelolab/ark-analysis/blob/master/ark/phenotyping/som_utils_test.py#L1330

How is it passing if this part of the code got dropped?

[alex-l-kong]

@ngreenwald ok I see the problem now: mocked_create_fov_pixel_data never actually gets entered. Instead, the test enters the create_fov_pixel_data in som_utils.py. This means none of the assertions in mocked_create_fov_pixel_data actually get checked.

We'll need to find a way to get this mocked up correctly. My guess is it's because you're trying to mock a function that gets called internally by create_pixel_matrix (as opposed to mocking a function that you directly call in a test in som_utils_test.py). I'm working on addressing the new file norm file save location, I'll look into it there.

[ngreenwald]

Okay sounds good. When you get the mocking figured out, can you confirm that the test fails with the current version in master and is fixed by candace's addition here

[alex-l-kong]

@ngreenwald verified that the new mocked function is being entered and passed.