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

WIP: Combine KmerCountTables with Add() #73

Open
wants to merge 32 commits into
base: main
Choose a base branch
from

Conversation

Adamtaranto
Copy link
Collaborator

@Adamtaranto Adamtaranto commented Oct 5, 2024

  • Add counts from a second KmerCountTable
  • Add hash_to_kmer records from a second KmerCountTable
  • Add tests
  • Handle case adding table to itself

Closes #62

@Adamtaranto Adamtaranto self-assigned this Oct 5, 2024
@Adamtaranto
Copy link
Collaborator Author

Not sold on having a subtract method, might get rid of it.

@Adamtaranto Adamtaranto changed the title WIP: Add and Subtract Tables WIP: Combine KmerCountTables with Add() Oct 13, 2024
@Adamtaranto
Copy link
Collaborator Author

Ditched the subtract function.

Need to decide if we should allow adding a table to itself - this creates issues around borrowing the same object multiple times and is generally a bit of a pain.

Maybe just raise error if trying to add table to itself?

@Adamtaranto
Copy link
Collaborator Author

I'm giving up on trying to check if self and other are the same table. Just let it die with borrow error.

@Adamtaranto Adamtaranto requested a review from ctb October 14, 2024 04:18
@Adamtaranto Adamtaranto added the enhancement New feature or request label Oct 14, 2024
@ctb
Copy link
Contributor

ctb commented Oct 29, 2024

hah! over in #83 I created a _merge method that does the same thing. let me look...

@ctb
Copy link
Contributor

ctb commented Oct 29, 2024

I am wondering if we want to avoid doing merges in parallel fashion? The locking is going to add a lot of overhead I think.

@Adamtaranto
Copy link
Collaborator Author

Should I fully remove parallelisation from the function?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add and Subtract methods
2 participants