Skip to content
This repository was archived by the owner on Nov 8, 2024. It is now read-only.

fix: | on dicts is only available with Python-3.9+#66

Merged
rasendubi merged 1 commit intomainfrom
fix-dict-merge
Aug 28, 2024
Merged

fix: | on dicts is only available with Python-3.9+#66
rasendubi merged 1 commit intomainfrom
fix-dict-merge

Conversation

@rasendubi
Copy link
Collaborator


labels: mergeable

Fixes: #issue

Motivation and Context

| on dicts is only available with Python-3.9+. Using Python-3.8 or lower, tests fail

Description

Replace | with unpacking.

How has this been tested?

Run tests with Python 3.8.3

Copy link
Member

@leoromanovsky leoromanovsky left a comment

Choose a reason for hiding this comment

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

confirmed that this produces equivalent output, thank you for finding it

Copy link

@schmit schmit left a comment

Choose a reason for hiding this comment

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

Good catch!

Copy link
Contributor

@aarsilv aarsilv left a comment

Choose a reason for hiding this comment

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

Nice spot!

@rasendubi rasendubi merged commit 4e88795 into main Aug 28, 2024
@rasendubi
Copy link
Collaborator Author

I thought that merging with ** is throws on duplicate keys, and this fix might interfere with FF-3106 (because FF-3106 duplicates bool attributes), so I implemented a fix: #67

Turns out, the order is defined (later values override previous ones) and is the same as for | operator, so these two issues are independent and don't amplify one another.

@rasendubi rasendubi deleted the fix-dict-merge branch August 28, 2024 11:38
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants