Deduplicate metadata download boilerplate#1027
Merged
Merged
Conversation
The same download-and-cache block (build cache path, skip if present, `_download` from the pertpy example-data server) was repeated seven times in `_cell_line.py` and again in `_moa.py` and `_drug.py`. Extract it into a single `MetaData._download_metadata` staticmethod next to `_warn_unmatch`, centralizing the base URL and cache-skip logic. The pharmgkb branch stays inline since it downloads a zip and renames the extracted file with the existence check on a different file. Also collapse the threefold source dispatch in `Drug.annotate`/`Drug.lookup` now that the lazy-load is uniform. Add network-free unit tests for the new helper covering URL construction and the cached-file skip. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #1027 +/- ##
==========================================
+ Coverage 77.95% 77.98% +0.02%
==========================================
Files 50 50
Lines 6587 6554 -33
==========================================
- Hits 5135 5111 -24
+ Misses 1452 1443 -9
🚀 New features to boost your workflow:
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
The identical download-and-cache block (build the cache path, skip if the file is already present,
_downloadfrom the pertpy example-data server) was repeated seven times in_cell_line.pyand again in_moa.pyand_drug.py.This PR extracts it into a single
MetaData._download_metadatastaticmethod, sitting next to the existing_warn_unmatchhelper on the shared superclass:Each
_download_*method drops from ~12 lines to 2–3, and the base URL / cache-skip logic now lives in one place.Details
_drug.pystays inline on purpose: it downloads a zip and renames the extracted file, with the existence check on a different file than the one downloaded, so folding it into the generic helper would change behavior.Drug.annotate/Drug.lookupnow that the lazy-load is uniform.Path/settings/_downloadimports from_cell_line.pyand_moa.py.Net change: +92 / −159 in the metadata package.
Tests
Added network-free unit tests (
tests/metadata/test_metadata.py) for the new helper: the existing metadata tests cover downloads end-to-end but require the network, and the refactor moved URL construction into one spot that now affects every download. The new tests mock_downloadand assert (a) the URL is built from the base and (b) a cached file is not re-fetched.The full
tests/metadata/suite (12 tests, real downloads) passes locally, confirming the refactor is behavior-preserving across all sources.🤖 Generated with Claude Code