-
Notifications
You must be signed in to change notification settings - Fork 109
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
Add Doc Strings to Config Files #465
base: main
Are you sure you want to change the base?
Add Doc Strings to Config Files #465
Conversation
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.
Very nice first draft!
In general, you need to homogeneize a bit (and remove useless words, I explained in the first class but you can apply it to all :)
@@ -47,6 +47,14 @@ def divide_chunks(array, n): | |||
|
|||
@dataclass | |||
class TGIModelConfig: | |||
""" | |||
This class provides a streamlined configuration for integrating with Text Generation Inference (TGI) endpoints. |
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.
Add a link to the doc of TGI, remove "This class"
This class provides a streamlined configuration for integrating with Text Generation Inference (TGI) endpoints. | ||
Attributes: | ||
inference_server_address (str, required): The endpoint address of the inference server hosting the model. |
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.
Remove "the"
@@ -58,7 +62,19 @@ def init_configs(self, env_config: EnvConfig): | |||
|
|||
|
|||
class AdapterModel(BaseModel): | |||
""" | |||
This class is designed to integrate adapter models with a pre-trained base model. |
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.
Remove "this class is"
@@ -71,7 +87,15 @@ def _create_auto_tokenizer(self, config: AdapterModelConfig, env_config: EnvConf | |||
) | |||
|
|||
def _create_auto_model(self, config: AdapterModelConfig, env_config: EnvConfig) -> AutoModelForCausalLM: | |||
"""Returns a PeftModel from a base model and a version fined tuned using PEFT.""" | |||
""" | |||
It returns a PeftModel from a base model and a version fined tuned using PEFT. |
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.
Removes "it"
…gEkbote/lighteval into Document-Custom-Model-Files
Co-authored-by: Clémentine Fourrier <[email protected]>
…gEkbote/lighteval into Document-Custom-Model-Files
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.
thanks for the doc ! Only need to adress a the few comment and this will be good to merge :)
Co-authored-by: Nathan Habib <[email protected]>
The docs for this PR live here. All of your documentation changes will be reflected on that endpoint. The docs are available until 30 days after the last update. |
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.
The syntax of the docstrings is not right. Please note, that the docstring parser is very strict with the syntax conventions: tabs, colons, parentheses,...
You have the syntax examples in the docs-builder repo: https://github.com/huggingface/doc-builder
Also the formatter raises errors: please run make style
to align with the linter/formatter conventions.
…gEkbote/lighteval into Document-Custom-Model-Files
I will update this PR as draft to fix the syntax errors and update the docstrings correctly as mentioned before. cc: @albertvillanova |
As discussed in #247, I've added docstrings to the config class of the required files and updated some of them for enhanced clarity.
Please let me know if any further corrections are needed and I will make the necessary changes.
partially fixes #247
cc: @clefourrier