-
-
Notifications
You must be signed in to change notification settings - Fork 693
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
tools: add pubmed tool #1044
base: main
Are you sure you want to change the base?
tools: add pubmed tool #1044
Conversation
6bad095
to
1f6e285
Compare
) | ||
|
||
// DefaultUserAgent defines a default value for user-agent header. | ||
const DefaultUserAgent = "github.com/tmc/langchaingo/tools/pubmed" |
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.
It think it would be better if this isn't exported and was set by the new function if the userAgent string is empty.
|
||
// New initializes a new PubMed Search tool with arguments for setting a | ||
// max results per search query and a value for the user agent header. | ||
func New(maxResults int, userAgent string) (*Tool, error) { |
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.
What do you think about having functions options here instead? Then we could have a default for maxResults and the ability to pass callbacks.Handler in the New function.
"github.com/tmc/langchaingo/tools/pubmed/internal" | ||
) | ||
|
||
func TestNew(t *testing.T) { |
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.
Does these test need to be mocked? It seems like the tests test the internal client. Maybe we can put this file in the internal package.
PR Checklist
memory: add interfaces for X, Y
orutil: add whizzbang helpers
).Fixes #123
).golangci-lint
checks.Fixes #1043