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

Add optional api_key parameter #362

Merged
merged 2 commits into from
Nov 13, 2023

Conversation

bettybas
Copy link
Contributor

This PR allows users to supply an optional Openai API key, this is useful for services that allow users to supply their own API keys (like web services).

@rlouf rlouf changed the title added optional openai api key parameter. Will use the environment var… Add optional api_key parameter Nov 13, 2023
def __init__(
self,
model_name: str,
api_key: Optional[str] = os.getenv("OPENAI_API_KEY"),
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Optional is equivalent to X | None in Python. Since a default value is provided this should not be typed as Optional.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Alright, the mypy precommit hook was complaining and that solved it. Will get it fixed correctly.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah right, it's because os.getenv can return None. Sorry bit new to python.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Makes sense

) -> Union[List[str], str]:
"""Generate a sequence that must be one of many options.
`
.. warning::
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why has this moved?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think formatter commit hook did this

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hmm doesn't mess with the formatting now.

@rlouf
Copy link
Member

rlouf commented Nov 13, 2023

Thank you for contributing! I have a couple of comments that will need to be addressed before merging.

@rlouf rlouf merged commit 4266fac into outlines-dev:main Nov 13, 2023
5 checks passed
@rlouf
Copy link
Member

rlouf commented Nov 13, 2023

Thank you for contributing!

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.

None yet

2 participants