Skip to content

fix: allow context.span_id as column name in pandas dataframe #7368

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

Merged
merged 1 commit into from
May 13, 2025

Conversation

RogerHYang
Copy link
Contributor

@RogerHYang RogerHYang commented Apr 30, 2025

Add support for context.span_id in annotations DataFrame validation

Changes

  • Added support for context.span_id column as an alternative to span_id in DataFrame validation
  • Added validation to prevent both span_id and context.span_id columns from being present simultaneously
  • Updated error messages to be more specific about which column is causing validation issues
  • Added comprehensive test cases for the new context.span_id functionality

Testing

  • Added new test cases covering:
    • Validation with context.span_id column
    • Invalid cases (empty strings, None values, non-string values)
    • Chunking functionality with context.span_id
    • Error handling for duplicate span ID columns

@github-project-automation github-project-automation bot moved this to 📘 Todo in phoenix Apr 30, 2025
@dosubot dosubot bot added the size:L This PR changes 100-499 lines, ignoring generated files. label Apr 30, 2025
@mikeldking mikeldking force-pushed the feat/annotations branch 2 times, most recently from 0750554 to beb1c9c Compare May 9, 2025 15:47
@mikeldking mikeldking requested review from a team as code owners May 9, 2025 15:47
@RogerHYang RogerHYang changed the base branch from feat/annotations to main May 9, 2025 23:04
@dosubot dosubot bot added size:XXL This PR changes 1000+ lines, ignoring generated files. and removed size:L This PR changes 100-499 lines, ignoring generated files. labels May 9, 2025
@RogerHYang RogerHYang force-pushed the allow-context-span-id-as-column-name branch from 59873b5 to 95674c6 Compare May 9, 2025 23:11
@dosubot dosubot bot added size:L This PR changes 100-499 lines, ignoring generated files. and removed size:XXL This PR changes 1000+ lines, ignoring generated files. labels May 9, 2025
Comment on lines +635 to +636
and not all(isinstance(x, str) for x in dataframe.index) # pyright: ignore[reportUnknownVariableType,reportUnknownMemberType]
):
Copy link
Contributor

Choose a reason for hiding this comment

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

pyright ignore? Do we need pyright here?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

pyright is used here for type checking TypedDict (so they would work somewhat like js). Unfortunately it means we need to ignore it every time we use pandas because pandas types are very lax.

@github-project-automation github-project-automation bot moved this from 📘 Todo to 👍 Approved in phoenix May 11, 2025
@RogerHYang RogerHYang merged commit ba1b9eb into main May 13, 2025
32 checks passed
@RogerHYang RogerHYang deleted the allow-context-span-id-as-column-name branch May 13, 2025 14:08
@github-project-automation github-project-automation bot moved this from 👍 Approved to ✅ Done in phoenix May 13, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
size:L This PR changes 100-499 lines, ignoring generated files.
Projects
Status: Done
Development

Successfully merging this pull request may close these issues.

2 participants