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

Remove intermediate _clustered/_consensus directories/files from pixel and cell clustering pipeline #586

Merged
merged 20 commits into from
Jun 10, 2022

Conversation

alex-l-kong
Copy link
Contributor

What is the purpose of this PR?

Closes #571. There is no need to create a separate pixel_mat_clustered directory/cell_mat_clustered file as the data is just the channel expression/pixel cluster count expression values with an additional pixel_som_cluster/cell_som_cluster attached. Ditto for the pixel_mat_consensus directory/cell_mat_consensus file. These should be removed from the pixel and cell clustering processes.

How did you implement your changes

We should overwrite the original data with the newly created data in the cluster_pixels/pixel_consensus_cluster (and similarly for the cell clustering functions).

Remaining issues

There are a few extra checks we need to include for validation if the user decides to rerun the clustering pipeline in the same session.

@review-notebook-app
Copy link

Check out this pull request on  ReviewNB

See visual diffs & provide feedback on Jupyter Notebooks.


Powered by ReviewNB

@alex-l-kong alex-l-kong self-assigned this Jun 3, 2022
@alex-l-kong
Copy link
Contributor Author

This branch is almost done, need to verify the outputs match up compared to the original.

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.

This looks great. Did you already run both notebooks on the HIV dataset to check that everything is working?

@alex-l-kong
Copy link
Contributor Author

@ngreenwald yes, I'm currently running one more test to be sure.

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 one leftover path change in the notebook

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 like some merge conflicts from the normalization PR I just put in

@alex-l-kong
Copy link
Contributor Author

@ngreenwald resolved

@ngreenwald ngreenwald merged commit 4cf115b into master Jun 10, 2022
@ngreenwald ngreenwald deleted the condence_cluster_data branch June 10, 2022 03:29
@srivarra srivarra added the enhancement New feature or request label Jun 16, 2022
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.

Remove intermediate pixel_mat_clustered/cell_mat_clustered directory/file
3 participants