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 for Issue #120 #180

Merged
merged 2 commits into from
May 2, 2022
Merged

Fix for Issue #120 #180

merged 2 commits into from
May 2, 2022

Conversation

Max-Bladen
Copy link
Collaborator

Replaced the variable df with contrib.df. As plotLoadings() iterates over each block, rather than setting df as the loading values, append these loading values to the contrib.df variable. Set the name of that contrib.df list item to that of the block and return contrib.df.

@Max-Bladen Max-Bladen self-assigned this Mar 8, 2022
@Max-Bladen
Copy link
Collaborator Author

Upon further inspection, this issue is not exclusively related to block.splsda objects. Running plotLoadings() on any multi-omics object (includes (s)pls, mint.splsda and interestingly splsda). Checks are failing due to the fact that they are seeking for a "data.frame". If we are to include all blocks' loadings values within a single output, we need a list of dataframes (as each dataframe has a different number of features, meaning it cannot be in a single dataframe).

Hence, the latest commit adjusts the values within test-plotLoadings.R such that they test pl_res against "list" rather than "data.frame" (lines: 13, 26, 45, 58)

@Max-Bladen Max-Bladen requested a review from aljabadi March 9, 2022 00:26
@Max-Bladen Max-Bladen added the bug-fix For PR's that address an Issue with `bug` label label Mar 9, 2022
@Max-Bladen Max-Bladen added the rapid-review for PRs which will take minimal time to review and close label May 2, 2022
@Max-Bladen
Copy link
Collaborator Author

While working on PR #205, I have reached the test-plotLoadings.R document. Due to the crucial changes that this PR makes to the return values of the plotLoadings() function, I will refrain from adjusting the test cases for this function until this PR has been merged. Otherwise, I would have to go back and redo these test cases in the future. When you get the chance, please review this PR @aljabadi

@aljabadi aljabadi merged commit d18f41c into master May 2, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug-fix For PR's that address an Issue with `bug` label rapid-review for PRs which will take minimal time to review and close
Projects
None yet
Development

Successfully merging this pull request may close these issues.

plotLoadings only returns loadings for a single block or input dataframe
2 participants