Skip to content

fix: use tuple instead of generator for DropDuplicates dictionary key#652

Open
yannisk2 wants to merge 1 commit intogenerative-computing:mainfrom
yannisk2:dropduplicatesfix
Open

fix: use tuple instead of generator for DropDuplicates dictionary key#652
yannisk2 wants to merge 1 commit intogenerative-computing:mainfrom
yannisk2:dropduplicatesfix

Conversation

@yannisk2
Copy link

@yannisk2 yannisk2 commented Mar 13, 2026

Misc PR

Type of PR

  • Bug Fix
  • New Feature
  • Documentation
  • Other

Description

The DropDuplicates rule was not dropping duplicates because the dictionary keys were generators and not tuples. This PR includes the following changes:

  • Fixes the DropDuplicates rule, by ensuring that the dictionary keys are tuples
  • Adds duplicates to the citations canned output test to catch regressions

Testing

  • Tests added to the respective file if code was changed
  • New code has 100% coverage if code as added
  • Ensure existing tests and github automation passes (a maintainer will kick off the github automation when the rest of the PR is populated)

The dictionary keys are generator expressions, which are not hashable and as a result duplicates were not detected. Using tuple() explicitly creates a hashable, comparable key to properly deduplicate records.

Closes generative-computing#651

Signed-off-by: Yannis Katsis <35782820+yannisk2@users.noreply.github.com>
@yannisk2 yannisk2 requested a review from a team as a code owner March 13, 2026 22:36
@github-actions
Copy link
Contributor

The PR description has been updated. Please fill out the template for your PR to be reviewed.

@mergify
Copy link

mergify bot commented Mar 13, 2026

Merge Protections

Your pull request matches the following merge protections and will not be merged until they are valid.

🟢 Enforce conventional commit

Wonderful, this rule succeeded.

Make sure that we follow https://www.conventionalcommits.org/en/v1.0.0/

  • title ~= ^(fix|feat|docs|style|refactor|perf|test|build|ci|chore|revert|release)(?:\(.+\))?:

Copy link
Collaborator

@frreiss frreiss left a comment

Choose a reason for hiding this comment

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

LGTM

@frreiss frreiss enabled auto-merge March 14, 2026 00:55
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