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.

Sign up for GitHub

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

Jump to bottom

Added a LangChain tooling bridge example #58

Closed
wants to merge 5 commits into from
Closed

Added a LangChain tooling bridge example #58

wants to merge 5 commits into from

Conversation

ElliotWood
Copy link
Contributor

Why are these changes needed?

As per discord feedback it was clear the LangChain integrations would be beneficial to this project. This example shows how to give your AutoGen agent the callability to use a LangChain toolkit/tool to solve the problem or query.

This means we can inherit the 35+ integrations from the LangChain community, but using the agent power of AutoGen.

Checks

@ElliotWood
Copy link
Contributor Author

@microsoft-github-policy-service agree company="LAB3"

@sonichi sonichi requested review from pcdeadeasy, ekzhu and a team October 1, 2023 14:21
@AaronWard
Copy link
Contributor

Great work @ElliotWood - this will be super useful, would be cool to have this ability in something like integration_utils.py

gagb
gagb requested changes Oct 1, 2023
View reviewed changes
Copy link
Collaborator

@gagb gagb left a comment

Choose a reason for hiding this comment

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

Great PR!

Feedback:

  • clarify the path of Test.txt. In my run, I had to place in cwd even when work_dir is set to coding. (Mac with python 3.11)
  • Spark example requires setting up key differently from circumference example. It would be great if the reader has to set everything in the beginning once.

pcdeadeasy
pcdeadeasy approved these changes Oct 2, 2023
View reviewed changes
Copy link
Contributor

@pcdeadeasy pcdeadeasy left a comment

Choose a reason for hiding this comment

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

Great example to bring in LangChain tools and agents into Autogen.

@gagb
Copy link
Collaborator

gagb commented Oct 2, 2023

@ElliotWood, lmk if the feedback makes sense before I can approve it to be added to the repo.

@wgong
Copy link

wgong commented Oct 4, 2023

@ElliotWood Great PR, can you share a high-level perspective on AutoGen vs LangChain: how they overlap and differ? Thanks.

@wgong
Copy link

wgong commented Oct 4, 2023

@ElliotWood posted the question to Bard, here is the answer, which is pretty good
image

@ElliotWood
Copy link
Contributor Author

@ElliotWood, lmk if the feedback makes sense before I can approve it to be added to the repo.

Hey just an FYI, im on vacation till the 20th I will push the updated feedback once ive returned, unless someone else wants to do it earlier ;)

@sonichi
Copy link
Contributor

sonichi commented Oct 12, 2023

@gagb can your comments be addressed in a separate PR or must be fixed in this PR? If the former, we can merge this PR and create an issue.

thinkall
thinkall reviewed Oct 12, 2023
View reviewed changes
Copy link
Collaborator

@thinkall thinkall left a comment

Choose a reason for hiding this comment

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

+1 for adding this into a utils file, but this can be done in another PR.

@gagb
Copy link
Collaborator

gagb commented Oct 14, 2023

@gagb can your comments be addressed in a separate PR or must be fixed in this PR? If the former, we can merge this PR and create an issue.

The pr works just needs more documentation clarity if you want a new user to be able to run it. I am okay your suggestion..

@sonichi sonichi enabled auto-merge October 14, 2023 21:42
@codecov-commenter
Copy link

codecov-commenter commented Oct 14, 2023 •
edited
Loading

Codecov Report

Merging #58 (59d9622) into main (ea8dec7) will not change coverage.
The diff coverage is n/a.

@@           Coverage Diff           @@
##             main      #58   +/-   ##
=======================================
  Coverage   43.31%   43.31%           
=======================================
  Files          17       17           
  Lines        2133     2133           
  Branches      481      481           
=======================================
  Hits          924      924           
  Misses       1126     1126           
  Partials       83       83
Flag Coverage Δ
unittests 43.22% <ø> (ø)

Flags with carried forward coverage won't be shown. Click here to find out more.

@ElliotWood
Copy link
Contributor Author

Just found this from Langchain side
https://python.langchain.com/docs/modules/agents/tools/tools_as_openai_functions
Could provide similar functionality

@sonichi
Copy link
Contributor

sonichi commented Oct 15, 2023

@gagb can your comments be addressed in a separate PR or must be fixed in this PR? If the former, we can merge this PR and create an issue.

The pr works just needs more documentation clarity if you want a new user to be able to run it. I am okay your suggestion..

Would you like to create an issue after this PR is merged?

@sonichi
Copy link
Contributor

sonichi commented Oct 16, 2023

The merging is blocked by "push cannot contain secrets" even though I've removed the keys in the PR. @ElliotWood do you think creating a new PR without secrets is the right solution?

@AaronWard
Copy link
Contributor

@sonichi https://docs.github.com/en/enterprise-cloud@latest/code-security/secret-scanning/push-protection-for-repositories-and-organizations#bypassing-push-protection-for-a-secret

@gagb
Copy link
Collaborator

gagb commented Oct 16, 2023

@gagb can your comments be addressed in a separate PR or must be fixed in this PR? If the former, we can merge this PR and create an issue.

The pr works just needs more documentation clarity if you want a new user to be able to run it. I am okay your suggestion..

Would you like to create an issue after this PR is merged?

Yes, should I wait for merge or create now?

@sonichi sonichi mentioned this pull request Oct 16, 2023
Merged
3 tasks
@ElliotWood
Copy link
Contributor Author

ElliotWood commented Oct 16, 2023 •
edited
Loading

The merging is blocked by "push cannot contain secrets" even though I've removed the keys in the PR. @ElliotWood do you think creating a new PR without secrets is the right solution?

Will do mate, I'm on vacation until the 20th so I don't have my laptop I will need to do it when i get back, apologies

@sonichi
Copy link
Contributor

sonichi commented Oct 16, 2023

@gagb can your comments be addressed in a separate PR or must be fixed in this PR? If the former, we can merge this PR and create an issue.

The pr works just needs more documentation clarity if you want a new user to be able to run it. I am okay your suggestion..

Would you like to create an issue after this PR is merged?

Yes, should I wait for merge or create now?

create after #263 is merged. And this PR will be closed.

@sonichi sonichi closed this in #263 Oct 16, 2023
auto-merge was automatically disabled October 16, 2023 23:32

Pull request was closed

jackgerrits pushed a commit that referenced this pull request Oct 2, 2024
@ekzhu
Improve assistant and chess examples to make them more robust and bet…
37cc6bc 
…ter presentation. (#58)

* Improve assistant and chess examples to make them more robust and better presentation.

* type dep

* format
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@thinkall thinkall thinkall left review comments

@sonichi sonichi sonichi left review comments

@pcdeadeasy pcdeadeasy pcdeadeasy approved these changes

@gagb gagb gagb approved these changes

@ekzhu ekzhu Awaiting requested review from ekzhu

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
8 participants
@ElliotWood @AaronWard @gagb @wgong @sonichi @codecov-commenter @thinkall @pcdeadeasy