Skip to content

Conversation

KennethEnevoldsen
Copy link
Contributor

@KennethEnevoldsen KennethEnevoldsen commented Jul 6, 2025

  • 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 is still a few left in AbsEncoder)

- 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
@KennethEnevoldsen KennethEnevoldsen requested a review from Copilot July 6, 2025 20:26
Copy link

@Copilot Copilot AI left a 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 expected ModelMeta 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 the get_model_meta module may be confusing; consider importing ModelMeta directly from its defining module (mteb.models.model_meta) for clarity.
from mteb.models.get_model_meta import ModelMeta, get_model_metas

@KennethEnevoldsen KennethEnevoldsen merged commit cddde06 into v2.0.0 Jul 9, 2025
9 checks passed
@KennethEnevoldsen KennethEnevoldsen deleted the refactor_models branch July 9, 2025 09:51
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.

2 participants