fix: add .get() for optional values in config instead of dict access #118
Conversation
WalkthroughThe update simplifies the logic in several functions related to callsets by changing the way Changes
Sequence Diagram(s)sequenceDiagram
participant U as Caller
participant F as Function (get_callset / others)
U->>F: Invoke callset function
alt Check rename-contigs
F->>F: If rename-contigs is True then proceed with renaming
else
F->>F: Skip renaming process
end
alt Check genome-build
F->>F: If genome-build is "grch37" (with default "grch38") then adjust processing
else
F->>F: Use default settings (grch38)
end
F-->>U: Return processed callset
Poem
Tip ⚡💬 Agentic Chat (Pro Plan, General Availability)
📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
workflow/rules/common.smk (1)
391-406: Improved safety for nested configuration accessThe implementation properly handles optional configuration fields in nested dictionary access. This ensures the code behaves as expected even when these configuration options are missing.
One suggestion: Consider extracting repeated accesses like
config["variant-calls"][wildcards.callset]into a local variable to improve readability:-if config["variant-calls"][wildcards.callset].get( - "genome-build", "grch38" -) == "grch37" and config["variant-calls"][wildcards.callset].get( - "rename-contigs", False -): +callset_config = config["variant-calls"][wildcards.callset] +if callset_config.get("genome-build", "grch38") == "grch37" and callset_config.get( + "rename-contigs", False +):
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
workflow/rules/common.smk(4 hunks)
🔇 Additional comments (3)
workflow/rules/common.smk (3)
211-213: Robust dictionary access: Good use of .get() methodThis change properly implements dictionary's
.get()method with default values for the optional configuration fields. The explicit comparison toTrueensures the condition only evaluates to true when the value is explicitly set to True, avoiding potential issues with truthy values.
224-226: Using .get() for safer dictionary accessUsing
.get()with a default value for the optional "rename-contigs" and "genome-build" fields is a good defensive programming practice. This avoids KeyError exceptions when these configuration options are not set.
237-239: Consistent implementation of safer dictionary accessThe changes here maintain consistency with the previous changes, safely accessing optional configuration values using the
.get()method.
Co-authored-by: Famke Bäuerle <45968370+famosab@users.noreply.github.com>
Config field rename-contigs and genome-build are optional. With direct dictionary access the lookup fails when the value is not set in the config. Now the config dict is queried via .get().
Summary by CodeRabbit