Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add multiple clustering resolution functionality #1095

Merged
merged 19 commits into from
Mar 1, 2024

Conversation

camisowers
Copy link
Contributor

@camisowers camisowers commented Dec 12, 2023

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

What is the purpose of this PR?

Closes #1070.

How did you implement your changes

Add helper function to create a new clustering column in the cell table. Include in notebook 4 with a description of the necessary inputs.

Remaining issues

NA

@camisowers camisowers changed the title Cluster granularities Add multiple clustering resolution functionality Dec 12, 2023
@camisowers camisowers marked this pull request as ready for review December 19, 2023 19:46
Copy link

Check out this pull request on  ReviewNB

See visual diffs & provide feedback on Jupyter Notebooks.


Powered by ReviewNB

@camisowers camisowers requested review from srivarra and alex-l-kong and removed request for srivarra December 20, 2023 00:00
@camisowers camisowers self-assigned this Dec 20, 2023
@camisowers camisowers added the enhancement New feature or request label Dec 20, 2023
templates/4_Post_Clustering.ipynb Show resolved Hide resolved
templates/4_Post_Clustering.ipynb Show resolved Hide resolved
@@ -6,9 +6,10 @@
"metadata": {},
Copy link
Contributor

@alex-l-kong alex-l-kong Dec 20, 2023

Choose a reason for hiding this comment

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

Is there a better way to do this other than raw printing everything out? At the very least, I think this should be formatted so it lists a separate cluster name per row.


Reply via ReviewNB

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Not really an automated way to do this since the user needs to manually define the cluster grouping dictionary. I'll fix it so it prints like below.

0              CD4T
1              CD8T
2     CD14_monocyte
3             Bcell
4             other
5     M2_macrophage
6      immune_other
7     M1_macrophage
8               APC
9            stroma
10      endothelium
11    Myofibroblast
12       tumor_ck17
13       tumor_ecad
Name: cell_meta_cluster, dtype: object

Copy link
Contributor

@srivarra srivarra left a comment

Choose a reason for hiding this comment

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

Alex covered all the notebook stuff well, I just have a small suggestion for using Pandas' built in methods.

src/ark/phenotyping/post_cluster_utils.py Outdated Show resolved Hide resolved
@camisowers
Copy link
Contributor Author

camisowers commented Jan 11, 2024

The ark tests are still failing, but can you just review the newest changes so it's ready to go once we get that fixed? @alex-l-kong @srivarra

Copy link
Contributor

@srivarra srivarra 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 a question about adding an additional check.

src/ark/phenotyping/post_cluster_utils.py Outdated Show resolved Hide resolved
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.

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

@ngreenwald ngreenwald merged commit 8877213 into main Mar 1, 2024
18 checks passed
@ngreenwald ngreenwald deleted the cluster_granularities branch March 1, 2024 00:37
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add template for generating multiple clustering resolutions
4 participants