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

Fixed bugs related to library normalization with no_dedup, replicate formatting, and DESeq #127

Merged
merged 8 commits into from
Apr 18, 2024

Conversation

epehrsson
Copy link
Collaborator

@epehrsson epehrsson commented Apr 11, 2024

Changes

Fixes bugs related to library normalization with no_dedup option, replicate formatting, DESeq (single-sample group check and log2FC parameter). Also allows local library installation as workaround for installation issues in shared directory.

Issues

PR Checklist

(Strikethrough any points that are not applicable.)

  • This comment contains a description of changes with justifications, with any relevant issues linked.
  • Update docs if there are any API changes.
  • Update CHANGELOG.md with a short description of any user-facing changes and reference the PR number. Guidelines: https://keepachangelog.com/en/1.1.0/

@epehrsson epehrsson marked this pull request as draft April 11, 2024 13:12
@epehrsson epehrsson marked this pull request as ready for review April 11, 2024 13:15
@kelly-sovacool
Copy link
Member

kelly-sovacool commented Apr 11, 2024

@epehrsson can you update the changelog (CHANGELOG.md) under the development version header with a description of these changes?

You can use something like this (or copy-paste this directly)

## CARLISLE development version

- Bug fixes (#127, @epehrsson)
    - Removed single-sample group check.
    - Increase memory for DESeq.
    - Ensure control replicate number is an integer.
    - Fix FDR cutoff misassigned to log2FC cutoff.
    - Improve `no_dedup_nreads_genome` variable name.

## CARLISLE v2.5.0

@kelly-sovacool
Copy link
Member

We should update the docs here

### 2.2.2 Contrast Manifest (OPTIONAL)
This manifest will include sample information to performed differential comparisons.
An example contrast file:
| condition1 | condition2 |
| --- | --- |
| MOC1_siSmyd3_2m_25_HCHO | MOC1_siNC_2m_25_HCHO |

With something like this:

**Note**: you must have more than one sample per condition in order to perform differential analysis.
Please ensure each condition contains more than one sample.

Copy link
Member

@kelly-sovacool kelly-sovacool left a comment

Choose a reason for hiding this comment

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

Thanks for contributing this, @epehrsson! See my comments above for suggested changes

  • changelog update
  • docs update
  • remove commented-out code

I think @kopardev should also take a look since he has worked on this pipeline more than I have, as I may have missed something.

@kelly-sovacool
Copy link
Member

@kopardev Maybe now is the time to go ahead and create a container for the DESeq step so we can get rid of the code related to installing R packages? (#128)

@epehrsson
Copy link
Collaborator Author

To add more context, the commented-out code allows the user to use their own R library directory instead of the central pipeline directory. Without this edit, rule DESeq fails due to missing packages (within _diff_markdown.Rmd, setup chunk, the CARLISLE_HANDLE_PACKAGES call). In particular, it seems unable to load some ChIPSeeker dependencies. It also no longer handles ELBOW installation. However, the user must install some libraries manually with this edit.

@kelly-sovacool
Copy link
Member

@epehrsson I chatted with Vishal and he agrees we should switch to using containers. For now, I think the best thing to do would be undo the changes related to the R package installation / lib path code. In a separate PR I'll get rid of that code and add a container.

@kelly-sovacool kelly-sovacool self-requested a review April 15, 2024 16:32
@kelly-sovacool kelly-sovacool merged commit bb5c9ec into CCBR:main Apr 18, 2024
1 check passed
@epehrsson epehrsson deleted the no_dedup branch July 15, 2024 20:11
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
2 participants