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

MRG: remove BuildParams, filter via manifest / Select approaches #127

Merged
merged 17 commits into from
Oct 24, 2024

Conversation

bluegenes
Copy link
Collaborator

@bluegenes bluegenes commented Oct 22, 2024

This PR replaces the BuildParams manual hashing + filtering with manifest filtering and selection (e.g. of moltypes) via a Select framework. This is more in tune with the approaches in sourmash core. I did add MultiSelect here, since we want to keep all templates that match sets of selection parameters.

Internal improvements to the framework introduced in 0.4.0:

  • All of the parameter parsing + checks from BuildParams/BuildParamsSet is now in BuildRecord/BuildCollection. We now parse the parameter string directly into a BuildCollection, rather than going through BuildParams as an intermediary.
  • To manage finding and filtering existing signatures, we now select on the BuildCollection directly via MultiSelect, e.g. for moltype filtering. We can also filter a manifest with another manifest by using PartialEq for BuildRecords. This replaces the prior approach of keeping a BuildParamsSet and hashing BuildParams manually for comparisons.
  • We continue to use sourmash core ComputeParameters to actually build template signatures.
  • Instead of handling DNA, protein as separate collections, manage them jointly by allowing selection on moltype and adding to DNA sigs when we have DNA, prot sigs when we have proteins. This reduces complexity of use and has the added benefit of easier addition of translated sigs if we want to add that in the future.
  • Fixes consider using sourmash core ComputeParameters instead of BuildParams #113

@bluegenes bluegenes changed the title EXP: remove BuildParams, filter manifest directly EXP: remove BuildParams, filter manifest via Select approach Oct 24, 2024
@bluegenes bluegenes marked this pull request as ready for review October 24, 2024 02:29
@bluegenes bluegenes changed the title EXP: remove BuildParams, filter manifest via Select approach EXP: remove BuildParams, filter via manifest / Select approaches Oct 24, 2024
@bluegenes bluegenes changed the title EXP: remove BuildParams, filter via manifest / Select approaches MRG: remove BuildParams, filter via manifest / Select approaches Oct 24, 2024
@bluegenes
Copy link
Collaborator Author

@ctb ready for review.

This restructure happened because I was doing some manual hashing of BuildParams, and realized that manifest filtering/PartialEq has that all baked in, and manifest filtering + selection would be much more in line with sourmash core approaches. No functionality changes here, all internal updates as described above.

@bluegenes bluegenes merged commit 2c898d7 into main Oct 24, 2024
1 check passed
@bluegenes bluegenes deleted the filter-manifest branch October 24, 2024 18:48
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.

consider using sourmash core ComputeParameters instead of BuildParams
2 participants