◐ Shell
reader mode source ↗
Skip to content

gh-87390: Fix starred tuple equality and pickling (alt)#92337

Merged
serhiy-storchaka merged 2 commits into
python:mainfrom
serhiy-storchaka:unpacked-tuple-eq-pickle
May 5, 2022
Merged

gh-87390: Fix starred tuple equality and pickling (alt)#92337
serhiy-storchaka merged 2 commits into
python:mainfrom
serhiy-storchaka:unpacked-tuple-eq-pickle

Conversation

@serhiy-storchaka

@serhiy-storchaka serhiy-storchaka commented May 5, 2022

Copy link
Copy Markdown
Member

It is a simpler alternative of #92249.

#87390

@serhiy-storchaka serhiy-storchaka added type-bug An unexpected behavior, bug, or error skip news topic-typing labels May 5, 2022
@serhiy-storchaka serhiy-storchaka mentioned this pull request May 5, 2022
Closed
JelleZijlstra
JelleZijlstra reviewed May 5, 2022
View reviewed changes

@JelleZijlstra JelleZijlstra left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Hide comment

Thanks, this is indeed a better solution.

Could you add tests for pickling starred aliases though? Your new tests only cover equality.

@Fidget-Spinner

Fidget-Spinner commented May 5, 2022

Copy link
Copy Markdown
Member

LGTM. But IMO this needs a news, and like Jelle mentioned more tests (maybe we can take the ones from Mathew's PR and credit him?)

@serhiy-storchaka

serhiy-storchaka commented May 5, 2022

Copy link
Copy Markdown
Member Author

The old tests already cover pickling. They were passed, because equality was broken in the same way as pickling. I fixed equality in #92335, but it made pickling tests failing.

Since it fixes not yet released code, there is no sense in adding a NEWS entry. It can only confuse readers.

@serhiy-storchaka

serhiy-storchaka commented May 5, 2022

Copy link
Copy Markdown
Member Author

Additional tests in Mathew's PR are for the third parameter of GenericAlias. That change is not included in this PR.

@serhiy-storchaka serhiy-storchaka merged commit 1ed8d03 into python:main May 5, 2022
@serhiy-storchaka serhiy-storchaka deleted the unpacked-tuple-eq-pickle branch May 5, 2022 17:16
@mrahtz

mrahtz commented May 7, 2022

Copy link
Copy Markdown
Contributor

Thanks Serhiy!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@JelleZijlstra JelleZijlstra JelleZijlstra left review comments

Assignees

No one assigned

Labels

topic-typing type-bug An unexpected behavior, bug, or error

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

5 participants

@serhiy-storchaka @Fidget-Spinner @mrahtz @JelleZijlstra @bedevere-bot