Repository navigation
fix: download readme and respect subfolder - #392
Conversation
Codecov Report✅ All modified and coverable lines are covered by tests.
🚀 New features to boost your workflow:
|
 |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
| if folder is not None and _has_valid_layout(folder / subfolder if subfolder else folder): | ||
| return folder |
There was a problem hiding this comment.
With force_download=False, a snapshot downloaded by the old code can contain all model files but no README.md. _has_valid_layout accepts it, so _resolve_folder returns before fetching the newly included README. model.language stays None even when the hub README declares a language.
Fetch missing README metadata without making a README mandatory for models that have none.
There was a problem hiding this comment.
The actionable fix is to avoid returning a valid cached snapshot when its model directory lacks README.md; let it fall through to snapshot_download so the README can be fetched. Keep _has_valid_layout unchanged, so models that genuinely have no README remain valid.
| if folder is not None and _has_valid_layout(folder / subfolder if subfolder else folder): | |
| return folder | |
| if folder is not None and _has_valid_layout(folder / subfolder if subfolder else folder): | |
| model_folder = folder / subfolder if subfolder else folder | |
| if (model_folder / "README.md").exists(): | |
| return folder |
This preserves the cache fast path when the README is already present, while allowing older cached snapshots to acquire it. Repositories without a README may be rechecked on later loads unless the cache layer records that the README is absent.
| with patch("model2vec.persistence.persistence.maybe_get_cached_model_path") as cache: | ||
| # Cached snapshot without the subfolder files | ||
| cache.return_value = tmp_path / "other" | ||
| model = StaticModel.from_pretrained("my_org/haha", subfolder="subfolder") |
There was a problem hiding this comment.
test_hub_subfolder_loading patches an incomplete cache, but StaticModel.from_pretrained defaults to force_download=True. The cache patch is never used, so the test would pass even if the new cache-layout check were removed.
Pass force_download=False and assert that the cache was checked before the download.
1 participant
We didn't respect the subfolder argument any more and also didn't download the README, so the language field was not filled. Caught by integration tests.