Skip to content

Automatically handle the group_by_column field on the ArrowChart#476

Merged
palewire merged 1 commit intochekos:mainfrom
palewire:main
Oct 21, 2025
Merged

Automatically handle the group_by_column field on the ArrowChart#476
palewire merged 1 commit intochekos:mainfrom
palewire:main

Conversation

@palewire
Copy link
Copy Markdown
Collaborator

No description provided.

@palewire palewire requested a review from Copilot October 21, 2025 17:31
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

This PR refactors the ArrowChart class to automatically derive the group-by-column feature flag from the presence of a groups_column value, rather than requiring it to be set explicitly. The group_by_column boolean field is removed in favor of inferring this setting from whether groups_column is None or not.

  • Replaced explicit group_by_column boolean field with automatic derivation from groups_column presence
  • Added new groups_column field to store the column name for grouping arrows
  • Updated serialization to set group-by-column flag based on groups_column being non-None

Reviewed Changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated no comments.

File Description
datawrapper/charts/arrow.py Removed group_by_column field, added groups_column field, and updated serialization/deserialization logic to automatically handle the group-by-column feature flag
tests/integration/test_arrow_chart.py Updated tests to use groups_column parameter and verify its value instead of the removed group_by_column boolean

Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.

@palewire palewire merged commit 965951a into chekos:main Oct 21, 2025
7 checks passed
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