Skip to content

Conversation

@hassiebp
Copy link
Contributor

@hassiebp hassiebp commented Feb 10, 2025

Important

Handle NaN values in EventSerializer.default by returning None.

  • Behavior:
    • In EventSerializer.default, handle float NaN values by returning None.

This description was created by Ellipsis for 85bc086. It will automatically update as commits are pushed.

Greptile Summary

Disclaimer: Experimental PR review

Adds handling for NaN (Not a Number) float values in the EventSerializer class by converting them to None during JSON serialization, since NaN values are not valid in JSON.

  • Added NaN handling in langfuse/serializer.py by converting NaN floats to None
  • Missing test coverage for the new NaN handling functionality should be added to validate behavior
  • Consider adding docstring or comment explaining the NaN to None conversion rationale

Copy link
Contributor

@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

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

if np is not None and isinstance(obj, np.generic):
return obj.item()

if isinstance(obj, float) and math.isnan(obj):
Copy link
Contributor

Choose a reason for hiding this comment

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

Consider also handling Infinity (math.isinf) alongside NaN if consistent nulling of non-finite floats is desired.

Suggested change
if isinstance(obj, float) and math.isnan(obj):
if isinstance(obj, float) and (math.isnan(obj) or math.isinf(obj)):

@hassiebp hassiebp merged commit 02bb496 into main Feb 10, 2025
8 of 10 checks passed
@hassiebp hassiebp deleted the hassieb/lfe-3936-fix-nan branch February 10, 2025 10:00
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.

bug: np.nan in input dictionary causes Langfuse traces to fail

2 participants