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.

Sign up for GitHub

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

Jump to bottom

Add native Python consensus clustering process #839

Merged
merged 57 commits into from
Dec 7, 2022
Merged

Add native Python consensus clustering process #839

merged 57 commits into from
Dec 7, 2022

Conversation

alex-l-kong
Copy link
Contributor

@alex-l-kong alex-l-kong commented Nov 21, 2022 •
edited
Loading

What is the purpose of this PR?

Closes #831. Closes #852. Consensus clustering needs to be ported over to Python as part of the efforts to remove all R code.

How did you implement your changes

See #831. I decided not to use a fixture for test_pixel_consensus_cluster and test_cell_consensus_cluster since those functions already generate test datasets. When we port the entire pipeline over to OOP, then we'll think about creating test fixtures.

@alex-l-kong
Add initial implementation of consensus clustering class
ea5a80b
@alex-l-kong
Add initial implementation of pixel consensus clustering in
cc70d92
@alex-l-kong
Add initial implementation of cell consensus clustering in
fad138b
@alex-l-kong
Change call to scale_data
a5e30b1
@alex-l-kong
Remove inplace argument from zscore apply
d55c69c
@alex-l-kong
Add status updates to consensus clustering
f2abd5f
@alex-l-kong
Fix call to verify_in_list
c79202d
@alex-l-kong
Add some more comments to PixieConsensusCluster
433141f
@alex-l-kong
Documentation, PYCODESTYLE, and add cluster_type as parameter to the …
4631ca2 
…constructor of PixieConsensusCluster
@alex-l-kong
Bring out som_col and meta_col definitions to the constructor
0a38fe8
@alex-l-kong
More documentation
d03fd8a
@alex-l-kong
Ensure cluster variable is type hinted to Callable
6ffd2d4
@alex-l-kong
Patch up feather save call
1b96a55
@alex-l-kong
Documentation OCD
72d4302
@alex-l-kong
Make sure fovs_list instead of fov_list is used
ff551bf
@alex-l-kong
Fix saving of consensus mapping to .feather
4d2a302
@alex-l-kong
Multiprocessing fixes
7163bfc
@alex-l-kong
Wrap consensus cluster call around helper function
f8b906b
@alex-l-kong
Adjust mapping scheme
58ccdf0
@alex-l-kong
Ensure meta cluster values are corrected to 1-indexing
a44e8ff
@alex-l-kong
Progress outputs
90260a2
@alex-l-kong
Make sure progress updates for multiprocessing print out every batch …
be0e619 
…and not just final
@alex-l-kong
Set random seed for consensus clustering
e9ee78d
@alex-l-kong
Port over changes in pixel clustering to cell clustering
0851e99
@alex-l-kong
Remove unnecessary ncores parameter
03650e4
@alex-l-kong
Use new multiprocess logic
0a33ad6
@alex-l-kong
Enforce consistent pixel_data_path naming
a66c1ec
@alex-l-kong
Typo: fov_list to fovs_list
8864f31
@alex-l-kong
Remove pandas copy warning from meta cluster adding 1 correction
00a2d6a
@alex-l-kong
Make sure cell data is being loaded in as feather and not csv
d8061bb
srivarra
srivarra reviewed Nov 22, 2022
View reviewed changes
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.

It looks good! Just a couple of small things.

ngreenwald
ngreenwald reviewed Nov 23, 2022
View reviewed changes
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.

I'll defer to @cliu72 on this one

@cliu72
Copy link
Contributor

cliu72 commented Nov 29, 2022

This looks good to me. Did you have any additional questions about testing? I didn't look at it that hard but it looks fine to me

@alex-l-kong
Copy link
Contributor Author

This looks good to me. Did you have any additional questions about testing? I didn't look at it that hard but it looks fine to me

Nope, everything else looks straightforward.

@alex-l-kong alex-l-kong requested review from ngreenwald and cliu72 and removed request for ngreenwald November 30, 2022 21:10
ngreenwald
ngreenwald approved these changes Nov 30, 2022
View reviewed changes
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
Copy link
Member

Will wait to merge this in until #823 is resolved

@ngreenwald ngreenwald mentioned this pull request Dec 7, 2022
Closed
@alex-l-kong
Copy link
Contributor Author

Pixel subsetting merging complete and tested.

@alex-l-kong alex-l-kong merged commit d454d09 into main Dec 7, 2022
@alex-l-kong alex-l-kong deleted the py_consensus branch December 7, 2022 18:23
@srivarra srivarra added the enhancement New feature or request label Jan 10, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@srivarra srivarra srivarra left review comments

@ngreenwald ngreenwald ngreenwald approved these changes

@cliu72 cliu72 Awaiting requested review from cliu72

Assignees

@alex-l-kong alex-l-kong

Labels
enhancement New feature or request
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Seed invalid integer Port pixel and cell consensus clustering process to Python
4 participants
@alex-l-kong @cliu72 @ngreenwald @srivarra