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

Prevent failure of R script in case of single RNA sample in a 'cohort' #477

Open
wants to merge 2 commits into
base: release-1.7.8
Choose a base branch
from

Conversation

nodrogluap
Copy link

@nodrogluap nodrogluap commented Oct 18, 2024

Would like to compare a single RNA sample against TCGA using Djerba, which means that the study cohort has a size of one.
Normally this should not be an issue, but R has a very annoying 'feature' where data frames with a single column are automatically converted to vectors. This breaks the find_expression.R evaluation logic, but is easily mitigated in this pull request by adding the drop=FALSE argument in three places where the study cohort data frame is sliced up in the code.

@iainrb iainrb changed the base branch from main to release-1.7.7 October 25, 2024 20:53
@iainrb iainrb self-requested a review October 25, 2024 20:54
Copy link
Collaborator

@iainrb iainrb left a comment

Choose a reason for hiding this comment

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

Code change seems reasonable, but it's resulted in some checksum failures in our unit tests. It may or may not be OK to update the checksums and proceed -- we'll investigate next week and get back to you. A few other things:

  • Our procedure is not to merge directly to the main branch. Instead, we have a release branch (currently release-1.7.7) which is merged to main just before a new release is tagged, and after internal testing and validation. I have changed the target branch to release-1.7.7.
  • Any Djerba PR should include a one-line description in the Unreleased section in CHANGELOG.md.
  • To support more collaborative development, we will look at making our tests more portable and documenting on ReadTheDocs.

Thanks and have a good weekend! 😃

@iainrb iainrb changed the base branch from release-1.7.7 to release-1.7.8 November 11, 2024 15:39
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