Skip to content

gh-153896: Deduplicate unhashable Literal params#153914

Merged
ZeroIntensity merged 5 commits into
python:mainfrom
Maxinho96:main
Jul 18, 2026
Merged

gh-153896: Deduplicate unhashable Literal params#153914
ZeroIntensity merged 5 commits into
python:mainfrom
Maxinho96:main

Conversation

@Maxinho96

@Maxinho96 Maxinho96 commented Jul 18, 2026

Copy link
Copy Markdown
Contributor

@bedevere-app

bedevere-app Bot commented Jul 18, 2026

Copy link
Copy Markdown

Most changes to Python require a NEWS entry. Add one using the blurb_it web app or the blurb command-line tool.

If this change has little impact on Python users, wait for a maintainer to apply the skip news label instead.

@python-cla-bot

python-cla-bot Bot commented Jul 18, 2026

Copy link
Copy Markdown

All commit authors signed the Contributor License Agreement.

CLA signed

@picnixz

picnixz commented Jul 18, 2026

Copy link
Copy Markdown
Member

#153913 was just open a few minutes ago, but maybe @sobolevn wants relinquish his right to the PR

@sobolevn

Copy link
Copy Markdown
Member

Sure, since this is the first contribution by @Maxinho96, I would prefer to merge their PR :)

@bedevere-app

bedevere-app Bot commented Jul 18, 2026

Copy link
Copy Markdown

Most changes to Python require a NEWS entry. Add one using the blurb_it web app or the blurb command-line tool.

If this change has little impact on Python users, wait for a maintainer to apply the skip news label instead.

@sobolevn

sobolevn commented Jul 18, 2026

Copy link
Copy Markdown
Member

⚽️ I am a huge ⚪️🔵 Lazio 🦅 fan

Oh no, closing! 😱
I am AS Roma fan 😱

I am joking, obviously (not about the Roma part) :)
Thanks for the PR! And welcome :)

Comment thread Lib/typing.py
Comment on lines +778 to 780
value_and_type_parameters = list(_value_and_type_iter(parameters))
deduplicated_parameters = tuple(p for p, _ in _deduplicate(value_and_type_parameters, unhashable_fallback=True))

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.

We ususally want lines under 80 charactesr, so I suggest that you rename value_and_type_parameters to a shorter name or you use the same format as Nikita:

    parameters = tuple(
        p
        for p, _ in _deduplicate(
            list(_value_and_type_iter(parameters)),
            unhashable_fallback=True,
        )
    )

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

For the future: how do I enforce the correct formatting? The pre-commit hook passed for some reason. I see it uses ruff, but if I run it one the codebase it changes all the files

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.

Unfortunately, this is a manual task =/ we do have some parts of the stdlib with ruff configs, but for historical reasons, we don't enforce style on old files (for maintenance and diff purposes) and only try to enforce them for new code (so code you add, or code you change around). See PEP-7/PEP-8 guidelines (while they are guidelines, sometimes we need to break them because it's not consistent).

What we aim for:

  • Minimal diff in general.
  • If possible PEP-7/PEP-8 style if this doesn't introduce too many unrelated changes.
  • If not psosible, keep consistent style.

@tomasr8

tomasr8 commented Jul 18, 2026

Copy link
Copy Markdown
Member

@Maxinho96 Make sure to add a NEWS entry as well :)

Comment thread Lib/test/test_typing.py Outdated
@Maxinho96

Copy link
Copy Markdown
Contributor Author

Oh no, closing! 😱 I am AS Roma fan 😱

@sobolevn lol what are the chances? I forgive you for your sins since you kindly let me contribute, thanks! 🙏🏻

@ZeroIntensity ZeroIntensity 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.

Thanks! This looks good to me.

I think backporting makes sense here. It's unlikely that people are relying on this in a way that would break code; if anything, people could have been running their own deduplication pass on __args__, but that won't be broken by this fix.

@github-project-automation github-project-automation Bot moved this from Todo to In Progress in Sprint Jul 18, 2026
@ZeroIntensity ZeroIntensity added needs backport to 3.13 bugs and security fixes needs backport to 3.14 bugs and security fixes needs backport to 3.15 pre-release feature fixes, bugs and security fixes labels Jul 18, 2026
@ZeroIntensity
ZeroIntensity merged commit a53b5b1 into python:main Jul 18, 2026
61 checks passed
@github-project-automation github-project-automation Bot moved this from In Progress to Done in Sprint Jul 18, 2026
@miss-islington-app

Copy link
Copy Markdown

Thanks @Maxinho96 for the PR, and @ZeroIntensity for merging it 🌮🎉.. I'm working now to backport this PR to: 3.13, 3.14, 3.15.
🐍🍒⛏🤖

@bedevere-app

bedevere-app Bot commented Jul 18, 2026

Copy link
Copy Markdown

GH-153955 is a backport of this pull request to the 3.15 branch.

@bedevere-app bedevere-app Bot removed the needs backport to 3.15 pre-release feature fixes, bugs and security fixes label Jul 18, 2026
@bedevere-app

bedevere-app Bot commented Jul 18, 2026

Copy link
Copy Markdown

GH-153956 is a backport of this pull request to the 3.14 branch.

@bedevere-app bedevere-app Bot removed the needs backport to 3.14 bugs and security fixes label Jul 18, 2026
@bedevere-app

bedevere-app Bot commented Jul 18, 2026

Copy link
Copy Markdown

GH-153957 is a backport of this pull request to the 3.13 branch.

@bedevere-app bedevere-app Bot removed the needs backport to 3.13 bugs and security fixes label Jul 18, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

Status: Done

Development

Successfully merging this pull request may close these issues.

5 participants