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

Suggestions for get_data function #13

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

standage
Copy link

The get_data function is small enough that it didn't take too long to understand what it's doing. However, there are several changes that should make it much clearer for future review/maintenance.

  • a few variable name replacements
    • thefile --> filename to clarify that it's a file name, rather than an already-opened file handle
    • position_reads --> run_index and read_type --> run_acc: the original variable names were unclear at best, and potentially misleading at worst. Made it more difficult to understand the purpose of the code.
    • a few other minor changes
  • cut verbosity of code by using a dict of sets instead of a dict of lists; a set automatically handles duplicates, so you don't have to check that it's already there

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.

1 participant