Skip to content

Modernize type hints and clean up unused code #1928

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

Closed
wants to merge 9 commits into from

Conversation

ShorthillsAI
Copy link

@ShorthillsAI ShorthillsAI commented May 14, 2025

Description

Modernized type hints using Python 3.10+ syntax (e.g., list[str] instead of List[str]) and removed unused imports, variables, and functions to improve code clarity and maintainability.

Related Issues

[Reference any related issues or tasks that this pull request addresses.]

Proposed Changes

[List the specific changes made in this pull request.]

Checklist

  • I have tested these changes locally.
  • I have reviewed the code changes.
  • I have updated the documentation (if necessary).
  • I have added appropriate unit tests (if applicable).

Additional Notes

[Add any additional notes or context that may be helpful for the reviewer(s).]

@ShorthillsAI ShorthillsAI requested a review from a team as a code owner May 14, 2025 12:19
@SiddharthAnandShorthillsAI

@microsoft-github-policy-service agree [company="{ShorthillsAI}"]

@ShorthillsAI
Copy link
Author

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

@AlonsoGuevara
Copy link
Collaborator

Hi! Thanks for your submission!

I see your PR is doing two different things, one not necessarily aligned to the PR message.
Could you please divide this into different PRs?

Thanks!

@@ -12,6 +12,11 @@
create_openai_client,
create_openai_embeddings_llm,
)
from fnllm.gemini import (
Copy link
Collaborator

Choose a reason for hiding this comment

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

Is this intended to be included in your PR?
Gemini support is not available in fnllm, this will just break

@ShorthillsAI
Copy link
Author

Cancelling the PR, Will raise after some required changes.

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