langchain
langchain copied to clipboard
[test] Add integration_test for PandasAgent
- confirm creation
- confirm functionality with a simple dimension check.
The test now is calling OpenAI API directly, but learning from @vowelparrot that we’re caching the requests, so that it’s not that expensive. I also found we’re calling OpenAI api in other integration tests. Please lmk if there is any concern of real external API calls. I can alternatively make a fake LLM for this test. Thanks
Yeah, I was thinking of adding a scope=class fixture function which can mock up LLM or Agent for all test functions once together in the very beginning, but later I realized this kind of mock up doesn’t exist much yet. To make it right and reasonable, I changed my plan to start the mocking with LLM models first. When the proposal is aligned with everyone else, I’ll gradually add it to all tests from LLM, Chains to Agents here. Just decided to move forward in a longer but safer path. :)
On Wed, May 3, 2023 at 6:02 PM Davis Chase @.***> wrote:
@.**** commented on this pull request.
thanks @skcoirz https://github.com/skcoirz! quick question
In tests/integration_tests/agent/test_pandas_agent.py https://github.com/hwchase17/langchain/pull/4056#discussion_r1184440919:
- df: DataFrame
- dim: int
- class Config:
arbitrary_types_allowed = True
@.***(scope="function") +def data() -> TestData:
- dim = 4
- random_data = np.random.rand(dim, dim)
- df = DataFrame(random_data, columns=["name", "age", "food", "sport"])
- return TestData(df=df, dim=4)
+class TestPandasAgent:
out of curiosity, why implement as class instead of just having test functions?
— Reply to this email directly, view it on GitHub https://github.com/hwchase17/langchain/pull/4056#pullrequestreview-1412102550, or unsubscribe https://github.com/notifications/unsubscribe-auth/AO64MH2E2LG5RQ2NXRDDZ5LXEL52ZANCNFSM6AAAAAAXU2KOLQ . You are receiving this because you were mentioned.Message ID: @.***>
The benefit of this mock up is that we can avoid calling to the external APIs and at the same time make each LLM output more certain (the real model might generate different answer every a while even when the temperature is 0) I think this can be helpful for multi-tools collaboration functionality test where all tools will be called one by one with a series of predetermined LLM outputs.
This mock up won’t replace the real API call, but when our focus is on the tooling functionality inside of Agent, we will need the certainty to make sure all of them can be called in the expected way.
On Wed, May 3, 2023 at 6:33 PM Mike Wang @.***> wrote:
Yeah, I was thinking of adding a scope=class fixture function which can mock up LLM or Agent for all test functions once together in the very beginning, but later I realized this kind of mock up doesn’t exist much yet. To make it right and reasonable, I changed my plan to start the mocking with LLM models first. When the proposal is aligned with everyone else, I’ll gradually add it to all tests from LLM, Chains to Agents here. Just decided to move forward in a longer but safer path. :)
On Wed, May 3, 2023 at 6:02 PM Davis Chase @.***> wrote:
@.**** commented on this pull request.
thanks @skcoirz https://github.com/skcoirz! quick question
In tests/integration_tests/agent/test_pandas_agent.py https://github.com/hwchase17/langchain/pull/4056#discussion_r1184440919 :
- df: DataFrame
- dim: int
- class Config:
arbitrary_types_allowed = True
@.***(scope="function") +def data() -> TestData:
- dim = 4
- random_data = np.random.rand(dim, dim)
- df = DataFrame(random_data, columns=["name", "age", "food", "sport"])
- return TestData(df=df, dim=4)
+class TestPandasAgent:
out of curiosity, why implement as class instead of just having test functions?
— Reply to this email directly, view it on GitHub https://github.com/hwchase17/langchain/pull/4056#pullrequestreview-1412102550, or unsubscribe https://github.com/notifications/unsubscribe-auth/AO64MH2E2LG5RQ2NXRDDZ5LXEL52ZANCNFSM6AAAAAAXU2KOLQ . You are receiving this because you were mentioned.Message ID: @.***>
The benefit of this mock up is that we can avoid calling to the external APIs and at the same time make each LLM output more certain (the real model might generate different answer every a while even when the temperature is 0) I think this can be helpful for multi-tools collaboration functionality test where all tools will be called one by one with a series of predetermined LLM outputs. This mock up won’t replace the real API call, but when our focus is on the tooling functionality inside of Agent, we will need the certainty to make sure all of them can be called in the expected way. … On Wed, May 3, 2023 at 6:33 PM Mike Wang @.> wrote: Yeah, I was thinking of adding a scope=class fixture function which can mock up LLM or Agent for all test functions once together in the very beginning, but later I realized this kind of mock up doesn’t exist much yet. To make it right and reasonable, I changed my plan to start the mocking with LLM models first. When the proposal is aligned with everyone else, I’ll gradually add it to all tests from LLM, Chains to Agents here. Just decided to move forward in a longer but safer path. :) On Wed, May 3, 2023 at 6:02 PM Davis Chase @.> wrote: > @.**** commented on this pull request. > > thanks @skcoirz https://github.com/skcoirz! quick question > ------------------------------ > > In tests/integration_tests/agent/test_pandas_agent.py > <#4056 (comment)> > : > > > + df: DataFrame > + dim: int > + > + class Config: > + arbitrary_types_allowed = True > + > + > @.(scope="function") > +def data() -> TestData: > + dim = 4 > + random_data = np.random.rand(dim, dim) > + df = DataFrame(random_data, columns=["name", "age", "food", "sport"]) > + return TestData(df=df, dim=4) > + > + > +class TestPandasAgent: > > out of curiosity, why implement as class instead of just having test > functions? > > — > Reply to this email directly, view it on GitHub > <#4056 (review)>, > or unsubscribe > https://github.com/notifications/unsubscribe-auth/AO64MH2E2LG5RQ2NXRDDZ5LXEL52ZANCNFSM6AAAAAAXU2KOLQ > . > You are receiving this because you were mentioned.Message ID: > @.> >
sorry i still dont understand why having the test as a class instead of functions helps
this is partially on us - we need to better define how tests should be written, and make sure they are done consistently
I can change it to functions. No problem. Class is helpful when all test_functions need the same temporary data setup. We can discuss in a later PR when I have the code ready. With the real code, discussion will be easier.