angelolab / ark-analysis

Integrated pipeline for multiplexed image analysis
https://ark-analysis.readthedocs.io/en/latest/
MIT License
72 stars 25 forks source link

Mutiple tiled stitching tweaks #981

Closed camisowers closed 1 year ago

camisowers commented 1 year ago

If you haven't already, please read through our contributing guidelines before opening your PR

What is the purpose of this PR?

Closes #975. Adds ability to account for multiple tiles in ark stitching.

How did you implement your changes

Adjust the output of alpineer.load_utils.load_tiled_img_data to be a list of tuples. Loop through the various tuples, since each should indicate an individual tiled grid.

Also angelolab/alpineer#25 changed the error message for empty lists in verify_in_list, so the spatial_lda_utils_test was adjusted.

Remaining issues

camisowers commented 1 year ago

@srivarra Could use your input on how alpineer.misc_utils.verify_in_list now handles empty lists.

srivarra commented 1 year ago

@camisowers If either list (or both) are none, it immediately returns True.

camisowers commented 1 year ago

@camisowers If either list (or both) are none, it immediately returns True.

What was the reasoning there? Just curious because it looks like spatial_lda_utils_test.test_check_format_cell_table_args specifically tests empty lists to trigger an error.

srivarra commented 1 year ago

@camisowers

What was the reasoning there?

I think the reasoning had something to do with the mathematics of sets, since that's what we are effectively doing. The empty Set is a member of the empty Set.

srivarra commented 1 year ago

Looks like docs aren't building, there is a thread here with everyone experiencing the same issue: https://github.com/urllib3/urllib3/issues/2168. One solution is to cap urllib3 with urllib3<2 in the rtd-requirements.txt.

alex-l-kong commented 1 year ago

Looks like docs aren't building, there is a thread here with everyone experiencing the same issue: urllib3/urllib3#2168. One solution is to cap urllib3 with urllib3<2 in the rtd-requirements.txt.

Pinning requests==2.28.0 also worked for me.

srivarra commented 1 year ago

@ngreenwald Looks like the Merge Queue issue relates to coveralls not being able to report back in merge queues. As such I've disabled the expected coveralls report for now.