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

fix additional issues regarding regress out #483

Merged
merged 6 commits into from
May 3, 2023

Conversation

Ukyeon
Copy link
Contributor

@Ukyeon Ukyeon commented May 1, 2023

  1. fix to use X_pca for pca instead of the X in .obsm
  2. fix documentations in regress_out_chunk
  3. period after each line of documentation
  4. For the n_cores, set default to be None. And then use half of available core as the default within the code.
  5. timeit should be removed.
  6. Do deep copy for comparison of prev and after variables.
  7. Please note that there is no duplicate operation in linear regression. The function regress_out_chunk is called twice in this code because it is being used to perform two separate tasks.
    • 1st: is used to predict the effects of the variables that are being removed from the target variable y_new.
    • 2nd: is used to predict the residuals after removing the effects of all the variables that were removed from the target variable.

@@ -172,14 +172,14 @@ def test_pca():
preprocessor = Preprocessor()
preprocessor.preprocess_adata_seurat_wo_pca(adata)
adata = dyn.pp.pca(adata, n_pca_components=30)
assert adata.obsm["X"].shape[1] == 30
assert adata.obsm["X_pca"].shape[1] == 30
Copy link
Collaborator

Choose a reason for hiding this comment

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

you will need to update the pca_key in the pca function so that the default key is X_pca https://github.com/aristoteleo/dynamo-release/blob/master/dynamo/preprocessing/utils.py

def pca

@Xiaojieqiu Xiaojieqiu merged commit 7a3dc6b into aristoteleo:master May 3, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants