Skip to content

Conversation

@ToluClassics
Copy link
Contributor

No description provided.

@ToluClassics ToluClassics changed the title Add docstrings for openai and preprocessing modules Add Module Docstrings Jun 1, 2023
openai_key : str, optional
The OPEN AI key to use. Defaults to None.
openai_org : str, optional
The OPEN AI organization to use. Defaults to None.
Copy link

Choose a reason for hiding this comment

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

I would change this to The OPEN AI organization ID

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 see, that's still a string right?

Copy link

Choose a reason for hiding this comment

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

Yeah, it's just a good reminder to use the string ID rather than the name

@ToluClassics ToluClassics marked this pull request as draft June 1, 2023 12:44
@ToluClassics ToluClassics marked this pull request as ready for review June 2, 2023 01:24
@ToluClassics ToluClassics requested a review from kitsamho June 2, 2023 13:09
@OKUA1 OKUA1 requested review from OKUA1 and removed request for kitsamho June 3, 2023 20:06
Copy link
Collaborator

@OKUA1 OKUA1 left a comment

Choose a reason for hiding this comment

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

Please see the comments

Copy link
Collaborator

@iryna-kondr iryna-kondr left a comment

Choose a reason for hiding this comment

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

In addition to the mentioned comments, could you please adjusts most of the docstrings such that they are written in the third person, singular form (e.g. "Fits the model")?

@OKUA1 OKUA1 merged commit d3fb412 into BeastByteAI:main Jun 7, 2023
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.

4 participants