Skip to content

Conversation

@Chenglong-MS
Copy link
Collaborator

Support running models beyond open ai models

image

@Chenglong-MS Chenglong-MS changed the title Dev Support custom models Feb 12, 2025
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Copilot reviewed 6 out of 20 changed files in this pull request and generated 1 comment.

Files not reviewed (14)
  • api-keys.env.template: Language not supported
  • py-src/data_formulator/agents/agent_utils.py: Evaluated as low risk
  • py-src/data_formulator/agents/agent_data_rec.py: Evaluated as low risk
  • py-src/data_formulator/agents/agent_data_clean.py: Evaluated as low risk
  • py-src/data_formulator/agents/agent_generic_py_concept.py: Evaluated as low risk
  • py-src/data_formulator/agents/agent_data_filter.py: Evaluated as low risk
  • py-src/data_formulator/agents/agent_sort_data.py: Evaluated as low risk
  • py-src/data_formulator/agents/agent_concept_derive.py: Evaluated as low risk
  • py-src/data_formulator/agents/agent_py_concept_derive.py: Evaluated as low risk
  • py-src/data_formulator/agents/agent_data_transformation.py: Evaluated as low risk
  • src/views/EncodingShelfThread.tsx: Evaluated as low risk
  • py-src/data_formulator/agents/agent_data_load.py: Evaluated as low risk
  • py-src/data_formulator/agents/agent_code_explanation.py: Evaluated as low risk
  • py-src/data_formulator/agents/agent_data_transform_v2.py: Evaluated as low risk
Comments suppressed due to low confidence (3)

py-src/data_formulator/app.py:38

  • [nitpick] The class name 'Client' is too generic and could lead to confusion. It should be renamed to something more specific like 'LiteLLMClient'.
from data_formulator.agents.client_utils import Client

py-src/data_formulator/app.py:132

  • The function 'check_available_models' does not have any associated tests to verify its behavior. This function is critical as it checks the availability of models and should be covered by tests to ensure it works correctly under different scenarios.
@app.route('/check-available-models', methods=['GET', 'POST'])

src/app/dfSlice.tsx:695

  • The domain variable is assigned uniqueValues, but the comment suggests it should be Array.from(columnValues). Confirm if the intended behavior is to use unique values or all column values.
const domain = uniqueValues; //Array.from(columnValues);

app = Flask(__name__, static_url_path='', static_folder=os.path.join(APP_ROOT, "dist"))
CORS(app)

def get_client(model_config):
Copy link

Copilot AI Feb 12, 2025

Choose a reason for hiding this comment

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

The 'html.escape' function is used to escape all values in 'model_config', which might not be necessary or appropriate for all fields (e.g., 'api_key'). This could lead to unintended behavior if the API key or other fields contain characters that are escaped.

Copilot uses AI. Check for mistakes.
@Chenglong-MS Chenglong-MS requested a review from Copilot February 12, 2025 23:26
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Copilot reviewed 11 out of 26 changed files in this pull request and generated 3 comments.

Files not reviewed (15)
  • api-keys.env.template: Language not supported
  • py-src/data_formulator/agents/agent_utils.py: Evaluated as low risk
  • py-src/data_formulator/agents/agent_data_clean.py: Evaluated as low risk
  • py-src/data_formulator/agents/agent_generic_py_concept.py: Evaluated as low risk
  • py-src/data_formulator/agents/agent_py_concept_derive.py: Evaluated as low risk
  • py-src/data_formulator/agents/agent_data_filter.py: Evaluated as low risk
  • py-src/data_formulator/agents/agent_concept_derive.py: Evaluated as low risk
  • py-src/data_formulator/agents/agent_sort_data.py: Evaluated as low risk
  • py-src/data_formulator/agents/agent_data_load.py: Evaluated as low risk
  • py-src/data_formulator/agents/agent_data_transformation.py: Evaluated as low risk
  • py-src/data_formulator/agents/agent_code_explanation.py: Evaluated as low risk
  • src/views/EncodingBox.tsx: Evaluated as low risk
  • src/views/DataFormulator.tsx: Evaluated as low risk
  • src/app/dfSlice.tsx: Evaluated as low risk
  • py-src/data_formulator/agents/agent_data_rec.py: Evaluated as low risk
Comments suppressed due to low confidence (1)

py-src/data_formulator/app.py:55

  • The function should have test coverage to ensure that it correctly handles different model configurations and creates the Client instance with the appropriate parameters.
def get_client(model_config):

Comment on lines 210 to 212
print("\n=== Data transformation result ===>\n")
print(choice.message.content + "\n")

Copy link

Copilot AI Feb 12, 2025

Choose a reason for hiding this comment

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

Replace print statements with proper logging.

Suggested change
print("\n=== Data transformation result ===>\n")
print(choice.message.content + "\n")
logger.info("\n=== Data transformation result ===>\n")
logger.info(choice.message.content + "\n")

Copilot uses AI. Check for mistakes.
Comment on lines 221 to 223
print("\n=== Code blocks ===>\n")
print(code_blocks)

Copy link

Copilot AI Feb 12, 2025

Choose a reason for hiding this comment

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

Replace print statements with proper logging.

Suggested change
print("\n=== Code blocks ===>\n")
print(code_blocks)
logger.info("\n=== Code blocks ===>\n")
logger.info(code_blocks)

Copilot uses AI. Check for mistakes.
Comment on lines 249 to 251
print("\n=== Candidates ===>\n")
print(candidates)

Copy link

Copilot AI Feb 12, 2025

Choose a reason for hiding this comment

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

Replace print statements with proper logging.

Suggested change
print("\n=== Candidates ===>\n")
print(candidates)
logger.info("\n=== Candidates ===>\n")
logger.info(candidates)

Copilot uses AI. Check for mistakes.
@Chenglong-MS Chenglong-MS merged commit 3c5b58d into main Feb 13, 2025
6 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants