Skip to content
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

Fix multi-column segment key translation #848

Merged
merged 1 commit into from
Sep 19, 2022

Conversation

jamie256
Copy link
Contributor

Multi-column segment keys were mistranslated to v0 profiles because we incorrectly duplicated the first columns value in the SegmentTag in subsequent values.

Addresses #846

  • added some tests,
  • more logging
  • fix catching RuntimeError in deserialization of kll double/float in the translator to make round trip serialization tests work across whylogs versions v0 and v1.

…ogging, fix deserialization path with kll double/float
Copy link
Contributor

@TheMellyBee TheMellyBee left a comment

Choose a reason for hiding this comment

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

Lgtm!

@jamie256 jamie256 merged commit 065a2e5 into mainline Sep 19, 2022
@jamie256 jamie256 deleted the dev/jamie/segment_multi_key_fix branch September 19, 2022 22:49
goodwanghan pushed a commit to goodwanghan/whylogs that referenced this pull request Sep 20, 2022
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.

2 participants