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: allow empty input tables #140

Merged
merged 7 commits into from
Jul 31, 2024

Conversation

deliaBlue
Copy link
Collaborator

This PR closes #136 .

The changes in merge_tables.R are:

  • Add a new function get_table() that tries to read the input file and if empty, create a data frame made of one row with NAs in each column
  • Refactor the merge_tables() function to include the new function and, if an empty file was present, remove the row with a NA in the miRNA species column
  • Document both functions
  • Refactor the code (max. line length, space between operators...) according to the R style
  • Update the description message

@deliaBlue deliaBlue added the bug Something isn't working label Feb 11, 2024
@deliaBlue deliaBlue requested a review from uniqueg February 11, 2024 19:17
@deliaBlue deliaBlue self-assigned this Feb 11, 2024
@deliaBlue deliaBlue linked an issue Feb 11, 2024 that may be closed by this pull request
Copy link
Member

@uniqueg uniqueg 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 some minor issues :)

scripts/merge_tables.R Show resolved Hide resolved
scripts/merge_tables.R Outdated Show resolved Hide resolved
scripts/merge_tables.R Show resolved Hide resolved
scripts/merge_tables.R Outdated Show resolved Hide resolved
scripts/merge_tables.R Outdated Show resolved Hide resolved
scripts/merge_tables.R Outdated Show resolved Hide resolved
scripts/merge_tables.R Outdated Show resolved Hide resolved
scripts/merge_tables.R Outdated Show resolved Hide resolved
Copy link
Member

@uniqueg uniqueg left a comment

Choose a reason for hiding this comment

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

It's actually really nice with the linting and docstrings and all of that good stuff. Good job! 👍

It just would have been nice to keep the linting and other improvements separate from the fix that this PR was supposed to be about, especially as they now make up, like, 95% of the whole PR. I really don't even know how to name this PR properly.

@deliaBlue deliaBlue merged commit 13d1568 into dev Jul 31, 2024
8 checks passed
@deliaBlue deliaBlue deleted the 136-fix-merge_tablesr-fails-if-given-empty-input branch July 31, 2024 12:35
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging this pull request may close these issues.

fix: merge_tables.R fails if given empty input
2 participants