-
Notifications
You must be signed in to change notification settings - Fork 39
Fix cnv_discordant_read_calls: raise ValueError when no data found #781
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
base: master
Are you sure you want to change the base?
Conversation
This looks good, thank you @Jiya873. I am going to shadow this PR to enable the checks. If you updated |
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #781 +/- ##
==========================================
- Coverage 96.13% 96.07% -0.06%
==========================================
Files 47 47
Lines 4683 4689 +6
==========================================
+ Hits 4502 4505 +3
- Misses 181 184 +3 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
Hi @jonbrenas, I had indeed forgotten to push the updated test_cnv_data.py earlier. I've just pushed the changes now. Please let me know if everything looks good or if further updates are needed. |
Thank you @Jiya873. It looks like the tests don't like your monkeypatching. I will try to look at what seems to be the problem ...if you don't beat me to it. |
Not sure what state this PR is in? |
I think it has been abandoned. |
Thanks @jonbrenas . If there's no response after a short time (say, September) then I guess we'll close this. This PR can always be resurrected, if needs be. |
This pull request addresses issue #777 where calling
cnv_discordant_read_calls
with absent data was causing anIndexError
instead of the expectedValueError
. The changes include:Refactoring
cnv_discordant_read_calls
:The empty-data check is now moved outside the inner loop that iterates over the sample sets. If no dataset is found for a contig (or across all contigs), a ValueError is raised with an informative message.
Updated Tests:
The relevant tests in
test_cnv_data.py
have been leveraged to confirm that the expectedValueError
is raised in cases where data is absent (e.g., for non-existent sample sets or contigs).These changes ensure that the API behaves more predictably and makes debugging easier when data is missing.