Skip to content

Comments

Suggestions for get_data function#13

Open
standage wants to merge 1 commit intojohnsolk:masterfrom
standage:get_data
Open

Suggestions for get_data function#13
standage wants to merge 1 commit intojohnsolk:masterfrom
standage:get_data

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