-
Notifications
You must be signed in to change notification settings - Fork 2.4k
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
Refactor: Cleanup playground file #1155
Refactor: Cleanup playground file #1155
Conversation
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.
Fantastic PR and mostly LGTM, just have a few comments regd imports
phi/playground/operator.py
Outdated
from typing import List, Optional | ||
|
||
from phi.agent.agent import Agent, Tool, Toolkit, Function | ||
from phi.utils.log import logger |
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.
update import ordering
from phi.agent.session import AgentSession
from phi.utils.log import logger
phi/playground/playground.py
Outdated
class GetAgentSessionsResponse(BaseModel): | ||
session_id: Optional[str] = None | ||
title: Optional[str] = None | ||
from .routes import create_playground_routes |
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.
not a fan of relative routes, lets use phi.playground.routes
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.
more info: https://peps.python.org/pep-0008/#imports
|
||
from pydantic import BaseModel | ||
from phi.agent.agent import Agent, RunResponse, Tool, Toolkit, Function | ||
from phi.agent.agent import Agent | ||
from phi.playground.settings import PlaygroundSettings | ||
from phi.utils.log import logger |
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.
import ordering
phi/playground/routes.py
Outdated
from phi.agent.agent import Agent, RunResponse | ||
from phi.utils.log import logger | ||
from phi.agent.session import AgentSession | ||
from .operator import get_agent_by_id, get_session_title, format_tools |
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.
please use absolute imports
phi/playground/routes.py
Outdated
from fastapi.responses import StreamingResponse, JSONResponse | ||
|
||
from phi.agent.agent import Agent, RunResponse | ||
from phi.utils.log import logger |
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.
import ordering, phi.utils should be at the end
@@ -0,0 +1,133 @@ | |||
from typing import List, Optional, Generator, Dict, cast, Union | |||
import base64 |
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.
base64 should be at the top
@@ -0,0 +1,133 @@ | |||
from typing import List, Optional, Generator, Dict, cast, Union | |||
import base64 | |||
from fastapi import APIRouter, HTTPException, UploadFile |
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.
new line between std lib imports and 3rd party imports
phi/playground/routes.py
Outdated
from phi.playground.operator import format_tools, get_agent_by_id, get_session_title | ||
from phi.utils.log import logger | ||
|
||
from .schemas import ( |
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.
@manthanguptaa here we should do phi.playground.schemas
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.
nice catch
No description provided.