-
Notifications
You must be signed in to change notification settings - Fork 3
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
ENH: Optimise CLI Usage of ctree
#71
Conversation
Reviewer's Guide by SourceryThis PR optimizes the CLI usage of the cluster tree functionality by introducing a new class Sequence diagram for optimized CLI usage of cluster treesequenceDiagram
participant User
participant CLI
participant dvs_cli_par_ctree
participant HDF5DataStore
participant ClusterTree
User->>CLI: Run ctree command
CLI->>dvs_cli_par_ctree: Initialize with parameters
dvs_cli_par_ctree->>HDF5DataStore: Read sequences from HDF5
HDF5DataStore-->>dvs_cli_par_ctree: Return sequence data
dvs_cli_par_ctree->>ClusterTree: Construct cluster tree
ClusterTree-->>dvs_cli_par_ctree: Return PhyloNode
dvs_cli_par_ctree-->>CLI: Return cluster tree
CLI-->>User: Output tree to file
Updated class diagram for cluster tree classesclassDiagram
class ClusterTreeBase {
<<abstract>>
}
class DvsParCtreeMixin {
+_mash_dist(seq_arrays: Sequence[SeqArray])
}
class dvs_par_ctree {
+__init__(k: int, sketch_size: int | None, moltype: str, distance_mode: Literal["mash", "euclidean"], mash_canonical_kmers: bool | None, show_progress: bool, max_workers: int | None, parallel: bool)
+main(seqs: c3_types.SeqsCollectionType) : PhyloNode
}
class dvs_cli_par_ctree {
+__init__(seq_store: str | Path, limit: int | None, k: int, sketch_size: int | None, moltype: str, distance_mode: Literal["mash", "euclidean"], mash_canonical_kmers: bool | None, show_progress: bool, max_workers: int | None, parallel: bool)
+main(seq_names: list[str]) : PhyloNode
}
ClusterTreeBase <|-- dvs_par_ctree
ClusterTreeBase <|-- dvs_cli_par_ctree
DvsParCtreeMixin <|.. dvs_par_ctree
DvsParCtreeMixin <|.. dvs_cli_par_ctree
note for dvs_cli_par_ctree "New class introduced to optimize CLI usage by reading sequences directly from HDF5 storage."
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hey @rmcar17 - I've reviewed your changes and they look great!
Here's what I looked at during the review
- 🟡 General issues: 1 issue found
- 🟢 Security: all looks good
- 🟢 Testing: all looks good
- 🟢 Complexity: all looks good
- 🟢 Documentation: all looks good
Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
Pull Request Test Coverage Report for Build 11734163625Details
💛 - Coveralls |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
the issue caught by codacy should have been caught by a test, can you add one that executes that line of code
Summary by Sourcery
Enhancements:
dvs_par_ctree
class to use a mixin for shared functionality, improving code modularity and reusability.