Skip to content

Conversation

@ch-kr
Copy link
Contributor

@ch-kr ch-kr commented Aug 21, 2025

The current code within annotate_downsamplings fails if gen_anc_expr isn't specified, despite this being an optional argument.

TypeError: annotate_globals: keyword argument 'ds_gen_anc_counts': expected expression of type any, found NoneType: None

This PR fixes this issue and also adds tests for this function

@ch-kr ch-kr requested a review from a team as a code owner August 21, 2025 21:14
@ch-kr ch-kr requested a review from mike-w-wilson August 21, 2025 21:14
Copy link
Contributor

@mike-w-wilson mike-w-wilson left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

A minor comment but otherwise looks good -- thanks for fixing and adding the tests!

return t.annotate_globals(
downsamplings=downsamplings,
ds_gen_anc_counts=gen_anc_counts,
)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is strictly personal preference and obviously makes no difference in what happens but if something exists in both the if and else which is essentially what this is, I typically move it out just for maintainability/readability. This one is super minor but also results in less diff. Feel free to ignore if you like the other way

@ch-kr ch-kr merged commit 44bda06 into main Aug 22, 2025
6 checks passed
@ch-kr ch-kr deleted the kc/annotate_downsampling branch August 22, 2025 18:21
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants