Skip to content

fix(client): some minor code fixes... #1176

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

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

XenoAmess
Copy link

@XenoAmess XenoAmess commented Apr 28, 2025

Important

Fix default mutable arguments, improve exception handling, and add missing class method decorator in various files.

  • Behavior:
    • Fix default mutable arguments in create_prompt(), update_prompt(), observe_llama_index(), and get_llama_index_handler() in client.py by setting default to None and initializing inside the function.
    • Improve exception handling in jsonable_encoder() in jsonable_encoder.py by directly initializing the errors list with the caught exception.
  • Functions:
    • Add missing @classmethod decorator to flush() in openai.py.
  • Misc:
    • Rename generate_error_message_fern() and handle_fern_exception() to use Exception instead of Error in parse_error.py.
    • Change default value of exclude to None in auto_decorate_methods_with() in error_logging.py.

This description was created by Ellipsis for ec01658. You can customize this summary. It will automatically update as commits are pushed.

Greptile Summary

Disclaimer: Experimental PR review

This PR improves code quality and type safety in the Langfuse Python SDK by addressing mutable default arguments and enhancing error handling.

  • Fixed mutable default arguments in client.py by changing empty lists/dicts to None in methods like create_prompt() and update_prompt()
  • Added missing @classmethod decorator to flush() in langfuse/openai.py for proper class method behavior
  • Simplified error handling in jsonable_encoder.py by directly initializing errors list with caught exceptions
  • Updated type hints from Error to Exception in parse_error.py for more flexible error handling
  • Changed default value of exclude to None in error_logging.py to avoid mutable default argument issues

The changes follow Python best practices by preventing shared state issues with mutable defaults while maintaining existing functionality.

💡 (3/5) Reply to the bot's comments like "Can you suggest a fix for this @greptileai?" or ask follow-up questions!

Copy link

@greptile-apps greptile-apps bot left a comment

Choose a reason for hiding this comment

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

6 file(s) reviewed, 1 comment(s)
Edit PR Review Bot Settings | Greptile

Copy link

@greptile-apps greptile-apps bot left a comment

Choose a reason for hiding this comment

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

LGTM

6 file(s) reviewed, no comment(s)
Edit PR Review Bot Settings | Greptile

@XenoAmess XenoAmess force-pushed the feature-fix-typehints branch from d827960 to c3d05ce Compare April 28, 2025 06:04
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.

1 participant