-
Notifications
You must be signed in to change notification settings - Fork 464
[v2] Refactor models/ module #2886
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
Conversation
- refactored loading of models - now all ModelMeta are imported - fixed a few metadata issues due to missing imports - renamed private methods to `_{prev_name}` to indicate that they are private - renamed `models/overview.py` > `models/get_model_meta.py`` - fixed a few typing issues in the models module
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
This PR refactors the models/ module by centralizing model metadata handling, renaming private helpers, fixing import paths, and correcting typing and language codes.
- Consolidated MODEL_REGISTRY import into
get_model_meta.py
and removed bulk overview imports - Renamed loader/meta builder functions to private variants and added validation in
ModelMeta
- Updated wrappers to use private methods, fixed variable names, and corrected prompt/type annotations
- Adjusted documentation in README for script and CLI usage, and added prompt details
Reviewed Changes
Copilot reviewed 14 out of 14 changed files in this pull request and generated 2 comments.
Show a summary per file
File | Description |
---|---|
tests/test_models/model_loading.py | Updated import path for MODEL_REGISTRY |
mteb/models/sentence_transformer_wrapper.py | Renamed inputs to _inputs and updated loader return type |
mteb/models/model_meta.py | Renamed get_loader_name to _get_loader_name and added validation for name /revision |
mteb/models/model_implementations/amazon_models.py | Fixed language code format (use hyphen instead of underscore) |
mteb/models/model_implementations/init.py | Added dynamic registry builder for ModelMeta |
mteb/models/instruct_wrapper.py | Refactored instruction_template positioning and renamed loop variable to _inputs |
mteb/models/get_model_meta.py | Consolidated registry import, removed bulk overview imports, renamed meta builders |
mteb/models/abs_encoder.py | Added cast around similarity outputs and updated prompt annotation |
mteb/load_results/benchmark_results.py | Updated import to use get_model_meta module |
mteb/leaderboard/table.py | Updated import to use get_model_meta module |
mteb/evaluation/MTEB.py | Switched to private _model_meta_from_* functions |
mteb/evaluate.py | Switched to private _model_meta_from_* functions |
mteb/init.py | Refactored exports to get_model_meta module |
README.md | Reworked example usage, added <details> sections, and expanded docs |
Comments suppressed due to low confidence (3)
README.md:56
- The
<details>
block for "Running SentenceTransformer model with prompts" is not closed; add a matching</details>
before the next section to ensure proper collapse behavior.
<details>
mteb/models/model_implementations/init.py:40
- The dynamic registry builder isn’t covered by existing tests; consider adding a unit test to verify that
MODEL_REGISTRY
contains expectedModelMeta
entries.
MODEL_REGISTRY = {meta.name: meta for meta in get_all_model_meta_objects()}
mteb/load_results/benchmark_results.py:22
- [nitpick] Importing
ModelMeta
from theget_model_meta
module may be confusing; consider importingModelMeta
directly from its defining module (mteb.models.model_meta
) for clarity.
from mteb.models.get_model_meta import ModelMeta, get_model_metas
_{prev_name}
to indicate that they are privatemodels/overview.py
> `models/get_model_meta.py``