Add utilities to convert feature masks to cell masks and vice versa, and add optional cell mask output from segmentation#512
Conversation
…_features to __init__
…_features to __init__
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## RC_v1.6.x #512 +/- ##
=============================================
+ Coverage 63.53% 65.39% +1.86%
=============================================
Files 27 27
Lines 3842 4066 +224
=============================================
+ Hits 2441 2659 +218
- Misses 1401 1407 +6
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
Linting results by Pylint:Your code has been rated at 8.33/10 (previous run: 8.36/10, -0.03) |
JuliaKukulies
left a comment
There was a problem hiding this comment.
Very useful addition, @w-k-jones! I have tested this with our example notebooks and it works without problems. The design looks also good to me!
I am just wondering if there was a reason why we are not adding the capability to have either cell or track numbers in the mask (and limit the conversion from features to track to a one-direction conversion only). Same is true for filtered features. With the function that you have for converting features to cell numbers, it seems like an easy add to instead of the cell numbers use tracks or filtered features.
Also, what do you think about modifying one of our example notebooks in a way that we apply this new function. I think as of now, most notebooks so the tracking first and then the segmentation, but it would be fairly easy to change one example to the tracking first and then show how the returned mask in the segmentation contains the cell numbers. I am happy to help with such an example!
|
I have added functions to convert feature/cell masks to tracks, but no operation in the other direction is possible (individual features get merged together and become inseparable). I have also added an Ready for re-review when tests have passed @JuliaKukulies @freemansw1 |
JuliaKukulies
left a comment
There was a problem hiding this comment.
Thanks for the additions @w-k-jones, great job and so useful!! See my specific comments below. The main thing is that we need to either write clear documentation which extra steps need to be done to add the track column to the dataframe of tracked features, or the function needs to be changed so it works with the original output from the merge split function. I think the first option would be easier, but maybe the second one cleaner - what do you think?
| Parameters | ||
| ---------- | ||
| features : pd.DataFrame | ||
| A feature dataframe with cell values provided by tobac.linking_trackpy |
There was a problem hiding this comment.
Update this documentation to tracks provided by the merge split module in tobac
| Parameters | ||
| ---------- | ||
| features : pd.DataFrame | ||
| A feature dataframe with cell values provided by tobac.linking_trackpy |
| timestep that is time_padding off of the feature. Extremely useful when | ||
| converting between micro- and nanoseconds, as is common when using Pandas | ||
| dataframes. | ||
| return_cells: bool, optional (default: False) |
There was a problem hiding this comment.
Since you added the capability to directly go from features to tracks as well, should that also be an option in the segmentation function?
| datetime(2000, 1, 1, 0), datetime(2000, 1, 1, 2), periods=3 | ||
| ), | ||
| "cell": [1, 1, 1], | ||
| "track": [1, 1, 1], |
There was a problem hiding this comment.
The functions convert_feature_mask_to_track and convert_cell_mask_to_track require the format of the original Features pandas dataframe with an added column track it seems like. As in one of your example notebooks (MCS tracking in ICON simulations), the suggested workflow is:
merge_splits = merge_splits.merge_split_MEST(Track, dxy = dxy)
Track['track'] = merge_splits.feature_parent_track_id
which I think is good but it needs to be documented somewhere. From a user perspective, it is not clear whether to input the output of merge_splits.merge_split_MEST or do that extra step. I am making this comment here, because maybe we should include this in the test as well; to ensure the output format of merging and splitting can be used with this function.
|
@JuliaKukulies @w-k-jones any opposition to pushing this to v1.6.3? |
No problem from my side! But also: sorry that this got delayed because of my comments. The code works all well, so I am also fine with merging as is, and I could provide some documentation in the do strings tomorrow to clarify what the input data frame for the features to track conversion needs to look like. Does that sound reasonable @w-k-jones ? |
|
@JuliaKukulies Thanks for flagging that issue, I've got so used to appending the tracks output to the feature dataframe that I forgot it isn't like that by default 😅 @freemansw1 Two options I think, first is to roll back to just the feature/cell conversion utilities and merge those for v1.6.2, second is to delay to v1.6.3 and possibly update the merge/split code to be able to optionally return a feature dataframe rather than a dataset |
Haha, makes sense! I also like your workflow and it makes things easier.
No matter which of the two options we go for, I am definitely in favor of updating the merge/split code to optionally return a dataframe and make it consistent so it also contains all columns from the feature detection/segmentation/tracking output. @kelcyno are you OK with that? I think you had a function for this in some example but I couldn't find it anymore |
|
I'm fine with updating the merge/split code to add the optional output. I do have code to combine the columns. However, we should make that standalone so we have the combined and compressed dataset outside of merge/split. I can start this PR over the break. |
|
Given the complexities this has opened, I've updated the milestone to 1.6.3. I'll try to get 1.6.2 released in the next couple of days. |
Adds features requested in #508, along with postprocessing utilities to perform the conversion in both directions. Note that in the presence of stub cells, this operation cannot be performed perfectly.
Some additional features not currently included are conversion to track masks (although the reverse is not possible) and a function for simultaneously filtering features from both a feature dataframe and the corresponding feature mask.