langchain icon indicating copy to clipboard operation
langchain copied to clipboard

[test] Add integration_test for PandasAgent

Open skcoirz opened this issue 1 year ago • 4 comments

  • 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

skcoirz avatar May 03 '23 19:05 skcoirz

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: @.***>

skcoirz avatar May 04 '23 01:05 skcoirz

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: @.***>

skcoirz avatar May 04 '23 01:05 skcoirz

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

hwchase17 avatar May 04 '23 05:05 hwchase17

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.

skcoirz avatar May 04 '23 16:05 skcoirz