Skip to content

Fix #1160: Replace broad except Exception with specific exception typ…#1161

Merged
jonbrenas merged 3 commits intomalariagen:masterfrom
khushthecoder:GH1160-phenotypes-exception-handling
Mar 20, 2026
Merged

Fix #1160: Replace broad except Exception with specific exception typ…#1161
jonbrenas merged 3 commits intomalariagen:masterfrom
khushthecoder:GH1160-phenotypes-exception-handling

Conversation

@khushthecoder
Copy link
Copy Markdown
Contributor

Fix #1160: Replace broad except Exception with specific exception types in phenotypes.py

Fixes: #1160

Summary

phenotypes.py had 5 instances of except Exception that silently converted real bugs (TypeError, KeyError, AttributeError, etc.) into warnings, returning empty or partial data. This is dangerous for scientific analysis where data correctness is critical.

Changes

Site Fix
_load_phenotype_data() outer loop Narrowed to (KeyError, FileNotFoundError, IOError)
_create_phenotype_dataset() variant merge Removed redundant catch-all — KeyError and ValueError already caught above
phenotype_data() metadata fetch Removed try/except — let sample_metadata() errors propagate naturally
phenotype_data() query evaluation Narrowed to (pd.errors.UndefinedVariableError, SyntaxError)
phenotype_sample_sets() path check Narrowed to (KeyError, FileNotFoundError)

Design rationale

  • Fail loudly on unexpected errors — a TypeError or AttributeError is a real bug that should crash with a traceback, not get silently converted into a warning
  • Keep expected error handlingFileNotFoundError when a sample set has no phenotype data, KeyError when a sample set has no release mapping, and UndefinedVariableError for bad user queries are all expected and still handled gracefully
  • Net negative lines — the fix removes more code than it adds (6 insertions, 17 deletions)

…es in phenotypes.py

The code had 5 instances of except Exception that silently converted
real bugs into warnings, returning empty/partial data. This is dangerous
for scientific analysis where data correctness is critical.

Changes:
- Site 1 (_load_phenotype_data): Narrowed to (KeyError, FileNotFoundError, IOError)
- Site 2 (_create_phenotype_dataset): Removed redundant catch-all (KeyError
  and ValueError were already caught above)
- Site 3 (phenotype_data): Removed try/except around sample_metadata() —
  let errors from the upstream API propagate naturally
- Site 4 (phenotype_data query): Narrowed to
  (pd.errors.UndefinedVariableError, SyntaxError) for malformed queries
- Site 5 (phenotype_sample_sets): Narrowed to (KeyError, FileNotFoundError)
@jonbrenas
Copy link
Copy Markdown
Collaborator

Thanks @khushthecoder, phenotypes.py is a work in progress (or maybe dead code) as the phenotypic data doesn't exist yet. Making it work better is thus not a priority but I will look at your PR as it may come in handy later if the code actually gets used.

@khushthecoder
Copy link
Copy Markdown
Contributor Author

Thank you for the review and approval @jonbrenas! Really appreciate it. Feel free to merge whenever convenient.

@jonbrenas jonbrenas merged commit 61ae527 into malariagen:master Mar 20, 2026
8 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Bug: phenotypes.py swallows all exceptions via broad except Exception, silently masking data and logic errors

2 participants