[joint caller] add contig_recoding option to run_combiner #8861
Conversation
| @@ -459,6 +459,7 @@ def run_combiner(sample_paths: List[str], | |||
| target_records: int = CombinerConfig.default_target_records, | |||
| overwrite: bool = False, | |||
| reference_genome: str = 'default', | |||
| contig_recoding: Dict[str, str] = None, | |||
There was a problem hiding this comment.
Ah, thanks. Optional use to be optional (heh) if the default initializer is None, but that's no longer recommended:
A past version of this PEP allowed type checkers to assume an optional type when the default value is None, as in this code:
def handle_employee(e: Employee = None): ...
This would have been treated as equivalent to:
def handle_employee(e: Optional[Employee] = None) -> None: ...
This is no longer the recommended behavior. Type checkers should move towards requiring the optional type to be made explicit.
| @@ -486,12 +487,18 @@ def run_combiner(sample_paths: List[str], | |||
| Overwrite output file, if it exists. | |||
| reference_genome : :obj:`str` | |||
| Reference genome for GVCF import. | |||
| contig_recoding: :obj:`dict` of (:obj:`str`, :obj:`str`) | |||
There was a problem hiding this comment.
I used , optional, since that seems to be what we do elsewhere. Let me know if that's no longer preferred.
Or does optional just mean it has a default value, and I should say dict of ... or None, optional?
I don't think we're very consistent about this stuff. :-/
There was a problem hiding this comment.
I'm not entirely sure what the right practices are. I'll do a bit of research...
And yeah, we really aren't consistent.
No description provided.