Skip to content

bpo-44712: Replace "type(literal)" with corresponding builtin types - #27294

Merged
serhiy-storchaka merged 7 commits into
python:mainfrom
serhiy-storchaka:type-literal
May 8, 2022
Merged

bpo-44712: Replace "type(literal)" with corresponding builtin types#27294
serhiy-storchaka merged 7 commits into
python:mainfrom
serhiy-storchaka:type-literal

Conversation

@serhiy-storchaka

@serhiy-storchakaserhiy-storchaka commented Jul 22, 2021

Copy link
Copy Markdown
Member

Comment threadLib/ftplib.py Outdated
Comment threadLib/test/test_copyreg.py
Comment threadLib/test/test_zipimport.py Outdated
mtime = int(mtime)
else:
mtime = int(-0x100000000 + int(mtime))
mtime = int(mtime)

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.

This and the next seem like different types of changes.

Copy link
Copy Markdown
MemberAuthor

Choose a reason for hiding this comment

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

This is a very old code, written when time could be int. It is complicated and not completely correct.

This change is included in #19708, so it will be removed from this PR.

@bedevere-bot

Copy link
Copy Markdown

When you're done making the requested changes, leave the comment: I have made the requested changes; please review again.

And if you don't make the requested changes, you will be put in the comfy chair!

@terryjreedy

Copy link
Copy Markdown
Member

The type(x) replacements are a real improvement in readability. Consider removing other changes while adding a few ==/!= replacements with is/is not.

Comment threadLib/test/test_pprint.py
@github-actions

Copy link
Copy Markdown

This PR is stale because it has been open for 30 days with no activity.

@github-actionsgithub-actionsBot added the stale Stale PR or inactive for long period of time. label Aug 24, 2021
Comment threadLib/test/test_copyreg.py
Comment threadLib/test/test_copyreg.py
Comment threadLib/test/test_pprint.py
Comment threadLib/test/test_copyreg.py
Comment threadLib/test/test_zipimport.py Outdated
mtime = int(mtime)
else:
mtime = int(-0x100000000 + int(mtime))
mtime = int(mtime)

Copy link
Copy Markdown
MemberAuthor

Choose a reason for hiding this comment

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

This is a very old code, written when time could be int. It is complicated and not completely correct.

This change is included in #19708, so it will be removed from this PR.

@serhiy-storchaka
serhiy-storchaka marked this pull request as ready for review May 3, 2022 06:23
@serhiy-storchaka
serhiy-storchaka requested review from a team and ethanfurman as code ownersMay 3, 2022 06:23
@serhiy-storchakaserhiy-storchaka removed the stale Stale PR or inactive for long period of time. label May 3, 2022

@ronaldoussorenronaldoussoren left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

The changes to plistlib and build-installer.py look good to me.

Likewise for most other changes. This does change type checks from (effectively) type(value) is some_type to instance(value, some_type) and those are technically semantic changes. That said, the changes should have no visible behaviour change in practice.

I didn't look at the changes in tkinter, pydoc and xmlrpc, mostly due to lack of time: I didn't have to time to look deeply enough at the code to see if there are unexpected changes in behaviour.

Comment threadLib/cgitb.py
Comment threadLib/test/test_copyreg.py
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants

@serhiy-storchaka@bedevere-bot@terryjreedy@ronaldoussoren@rhettinger@the-knights-who-say-ni