Skip to content
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

Improving Docstring for UnifyClient in single_sign_on_tool.py #2

Open
KatoStevenMubiru opened this issue Aug 13, 2024 · 2 comments
Open

Comments

@KatoStevenMubiru
Copy link
Collaborator

Description:
The docstring for the UnifyClient class in unify_llm_tool/tools/single_sign_on_tool.py could be more informative and complete. Currently, it lacks detailed descriptions for the configs, secrets, and kwargs parameters.

My suggested Improvement:

Provide a more comprehensive docstring that clearly explains the purpose of each parameter and how they are used. Here's an example of an improved docstring:

class UnifyClient(Unify):
    """
    Unify client with extended initialization parameters.

    :param configs: Configuration key-value pairs. This can include settings like
                    default model or provider.
    :type configs: Dict[str, str], optional
    :param secrets: Secret key-value pairs, such as the API key for authentication.
    :type secrets: Dict[str, str], optional
    :param kwargs: Additional keyword arguments that are passed to the base Unify class.
                    These may include `api_key`, `endpoint`, `model`, and `provider`.
                    Values in `kwargs` will take precedence over those in `configs` and `secrets`.
    """

    def __init__(self, configs: dict = None, secrets: dict = None, **kwargs: dict):
        # ... existing code ...

This improved UnifyClient docstring to be more informative by including details about its purpose, parameters, and their priority. This can enhance readability and make the code easier to understand and maintain.

@Kacper-W-Kozdon
Copy link
Owner

Good suggestion. This might also end up being moved to a separate file instead of being lumped in with tools. I think it should be better to add a folder for connections specifically and add the client and the custom connection in there.

@KatoStevenMubiru
Copy link
Collaborator Author

Sounds good, agree that moving it to a separate file/folder for connections would be a good idea. 👍

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

No branches or pull requests

2 participants