Skip to content

Conversation

srivarra
Copy link
Contributor

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

What is the purpose of this PR?

Keeps the segmentation consistent across all FOVs in a cohort, even if a FOV does not have a particular phenotype / cluster when compared to others in its cohort.
Adds the fix for cell clustering, neighborhood clustering, and for Mantis ingestion.

How did you implement your changes

Created a new dataclass CellClusterMaskData which contains all the cell data, cluster column, segmentation column for a particular cohort (or all FOVs used for the cell table).

Remaining issues

Partially addresses #857.

@srivarra srivarra self-assigned this Jun 30, 2023
@review-notebook-app
Copy link

Check out this pull request on  ReviewNB

See visual diffs & provide feedback on Jupyter Notebooks.


Powered by ReviewNB

@srivarra srivarra added the bug Something isn't working label Jun 30, 2023
@srivarra srivarra marked this pull request as ready for review June 30, 2023 18:35
Copy link
Contributor

@camisowers camisowers left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks really nice, just a couple clarifying questions.

Copy link
Contributor

@alex-l-kong alex-l-kong left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good, nothing further to add to Cami's comments.

@srivarra srivarra requested a review from ngreenwald June 30, 2023 20:27
Copy link
Member

@ngreenwald ngreenwald left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good, just some questions about implementation

@srivarra srivarra requested a review from ngreenwald July 5, 2023 21:12
Copy link
Member

@ngreenwald ngreenwald left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good, once @cliu72 is happy can merge it in.

Copy link
Contributor

@cliu72 cliu72 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just a few small comments

@srivarra srivarra requested a review from cliu72 July 19, 2023 00:15
Copy link
Contributor

@cliu72 cliu72 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks Sri! Looks good to me, just one tiny comment and I think we're good to merge.

@srivarra srivarra requested a review from cliu72 July 20, 2023 15:55
Copy link
Contributor

@cliu72 cliu72 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sweet, looks good!

@srivarra srivarra added this pull request to the merge queue Jul 20, 2023
@srivarra srivarra removed this pull request from the merge queue due to a manual request Jul 20, 2023
@srivarra srivarra added this pull request to the merge queue Jul 20, 2023
Merged via the queue into main with commit e604956 Jul 20, 2023
@srivarra srivarra deleted the pixie/cluster_id branch July 20, 2023 19:05
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Cell mask generation assumes all FOVs have the same maximum cluster number
5 participants