Mutiple tiled stitching tweaks #981
Conversation
|
@srivarra Could use your input on how |
|
@camisowers If either list (or both) are none, it immediately returns |
What was the reasoning there? Just curious because it looks like |
| @@ -51,11 +51,11 @@ def test_check_format_cell_table_args(): | |||
| spu.check_format_cell_table_args(valid_df, invalid_markers1, None) | |||
| with pytest.raises(ValueError): | |||
| spu.check_format_cell_table_args(valid_df, invalid_markers2, None) | |||
| with pytest.raises(ValueError, match=r"List arguments cannot be empty"): | |||
| with pytest.raises(ValueError, match=r"empty"): | |||
There was a problem hiding this comment.
alpineer.misc_utils.verify_in_list will return True if the lists are empty, do you think it would be better to do something else? Here is the original PR for that change: angelolab/alpineer#25. Let me know your thoughts.
There was a problem hiding this comment.
We can discuss at pipeline today. I think it really just depends on what we're using verify_in_lists for and if empty lists are considered something we would want to know about.
There was a problem hiding this comment.
We could handle this by setting a flag to override the default set behavior of empty lists. Something like fail_on_empty which defaults to True.
Since I'd imagine empty lists aren't the desired behavior for many users, we could also completely change verify_in_list and verify_same_elements to fail immediately on discovery of an empty list.
There was a problem hiding this comment.
@srivarra @alex-l-kong For now, I'm going to check within the function for an empty list. We can make long term changes to alpineer later if needed.
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. |
|
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 |
Pinning |
| @@ -51,11 +51,11 @@ def test_check_format_cell_table_args(): | |||
| spu.check_format_cell_table_args(valid_df, invalid_markers1, None) | |||
| with pytest.raises(ValueError): | |||
| spu.check_format_cell_table_args(valid_df, invalid_markers2, None) | |||
| with pytest.raises(ValueError, match=r"List arguments cannot be empty"): | |||
| with pytest.raises(ValueError, match=r"empty"): | |||
There was a problem hiding this comment.
We could handle this by setting a flag to override the default set behavior of empty lists. Something like fail_on_empty which defaults to True.
Since I'd imagine empty lists aren't the desired behavior for many users, we could also completely change verify_in_list and verify_same_elements to fail immediately on discovery of an empty list.
|
@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. |
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_datato 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 thespatial_lda_utils_testwas adjusted.Remaining issues