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

fix type hint in PromptPipeline.__init__ #496

Merged
merged 1 commit into from
Jun 8, 2023

Conversation

g-simmons
Copy link
Contributor

Fixes #494

@LouisCastricato
Copy link
Contributor

Ah I assumed you'd change the doc string. Did you verify that this change doesn't break anything?

@g-simmons
Copy link
Contributor Author

Oh, I assumed it was the type hint that was incorrect 😅.

Do you think the change is correct? Or is Dict[str, Any] actually one of the expected input types for prompts?

Would I need to do anything other than running the tests to confirm this doesn't break anything?

@g-simmons
Copy link
Contributor Author

Why I thought this change was correct:

  • eval_prompts is documented to accept List[Dict[str,Any]] in train fn of trlx.py
  • it seems like the first element of prompts is expected to sometimes be a dict in PromptPipeline.__init__.

Copy link
Collaborator

@cat-state cat-state left a comment

Choose a reason for hiding this comment

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

LGTM! Thanks @g-simmons

@cat-state cat-state merged commit 404217b into CarperAI:main Jun 8, 2023
@g-simmons
Copy link
Contributor Author

🖖

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.

Type hint doesn't match documentation for prompts arg of PromptPipeline.__init__
3 participants