Skip to content
This repository has been archived by the owner on Aug 29, 2020. It is now read-only.

replace type(x) == y checks by isinstance(x, y) and clean up resulting code #120

Merged
merged 7 commits into from Mar 26, 2020
Merged

replace type(x) == y checks by isinstance(x, y) and clean up resulting code #120

merged 7 commits into from Mar 26, 2020

Conversation

ghost
Copy link

@ghost ghost commented Mar 15, 2020

Especially, replace things like type(x) == type([]) by isinstance(x, list).

Also do x is None special casing.

Using the following py3 script:

import pathlib
import re
import sys

simple = {
    r"\[\]": r"list",
    r"\(\)": r"tuple",
    r"\(\d+,\)": r"tuple",
    r"\{\}": r"dict",
    r"\d+": r"int",
    r"''": r"str",
    r'""': r"str",
    r"long\(\d+\)": r"long",
}

others = {
    r"type\((.*?)\)\s*==\s*type\(\s*None\s*\)": r"\1 is None",
    r"type\((.*?)\)\s*!=\s*type\(\s*None\s*\)": r"\1 is not None",
    r"type\((.*?)\)\s*==\s*(type\(.*?\))": r"isinstance(\1, \2)",
    r"type\((.*?)\)\s*!=\s*(type\(.*?\))": r"not isinstance(\1, \2)",
}

def replace_in_all_files(root, pat, sub):
    for path in root.glob("**/*.py"):
        with path.open() as file:
            data = file.read()
        new_data = pat.sub(sub, data)
        if data != new_data:
            with path.open("w") as file:
                file.write(new_data)

root = pathlib.Path(sys.argv[1])

for str_pat, sub in simple.items():
    replace_in_all_files(
        root,
        re.compile(rf"type\((.*?)\)\s*==\s*type\(\s*{str_pat}\s*\)"),
        rf"isinstance(\1, {sub})",
    )
    replace_in_all_files(
        root,
        re.compile(rf"type\((.*?)\)\s*!=\s*type\(\s*{str_pat}\s*\)"),
        rf"not isinstance(\1, {sub})",
    )

for str_pat, sub in others.items():
    replace_in_all_files(root, re.compile(str_pat), sub)

Especially, replace things like `type(x) == type([])` by `isinstance(list)`.

Also do `x is None` special casing.

Using the following py3 script:

import pathlib
import re
import sys

simple = {
    r"\[\]": r"list",
    r"\(\)": r"tuple",
    r"\(\d+,\)": r"tuple",
    r"\{\}": r"dict",
    r"\d+": r"int",
    r"''": r"str",
    r'""': r"str",
    r"long\(\d+\)": r"long",
}

others = {
    r"type\((.*?)\)\s*==\s*type\(\s*None\s*\)": r"\1 is None",
    r"type\((.*?)\)\s*!=\s*type\(\s*None\s*\)": r"\1 is not None",
    r"type\((.*?)\)\s*==\s*(type\(.*?\))": r"isinstance(\1, \2)",
    r"type\((.*?)\)\s*!=\s*(type\(.*?\))": r"not isinstance(\1, \2)",
}

def replace_in_all_files(root, pat, sub):
    for path in root.glob("**/*.py"):
        with path.open() as file:
            data = file.read()
        new_data = pat.sub(sub, data)
        if data != new_data:
            with path.open("w") as file:
                file.write(new_data)

root = pathlib.Path(sys.argv[1])

for str_pat, sub in simple.items():
    replace_in_all_files(
        root,
        re.compile(rf"type\((.*?)\)\s*==\s*type\(\s*{str_pat}\s*\)"),
        rf"isinstance(\1, {sub})",
    )
    replace_in_all_files(
        root,
        re.compile(rf"type\((.*?)\)\s*!=\s*type\(\s*{str_pat}\s*\)"),
        rf"not isinstance(\1, {sub})",
    )

for str_pat, sub in others.items():
    replace_in_all_files(root, re.compile(str_pat), sub)
Copy link
Member

@Hoikas Hoikas left a comment

Choose a reason for hiding this comment

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

It looks like Cyan copy and pasted several anti-patterns that could be further cleaned up.

Python/GiraCave1.py Outdated Show resolved Hide resolved
Python/bhroBahroYeeshaCave.py Outdated Show resolved Hide resolved
Python/bhroBahroYeeshaCave.py Outdated Show resolved Hide resolved
Python/clftRS.py Outdated Show resolved Hide resolved
Python/clftRS.py Outdated Show resolved Hide resolved
Python/xTakableClothing.py Outdated Show resolved Hide resolved
Python/xTakableClothing.py Outdated Show resolved Hide resolved
Python/xTakableClothing.py Outdated Show resolved Hide resolved
Python/xTakableClothing.py Outdated Show resolved Hide resolved
Python/xTakableClothing.py Outdated Show resolved Hide resolved
Copy link
Member

@Hoikas Hoikas left a comment

Choose a reason for hiding this comment

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

It looks like Cyan copy and pasted several anti-patterns that could be further cleaned up.

@ghost
Copy link
Author

ghost commented Mar 16, 2020

All comments addressed. Most of them were some easy regex but had to make sure they caught all your comments.

return 0
else:
PtDebugPrint("ERROR: clftYeeshaPage08: Error trying to access the Vault. Can't access YeeshaPageChanges chronicle." )
PtDebugPrint("ERROR: clftYeeshaPage08: Error trying to access the Chronicle psnlSDL. psnlSDL = %s" % ( psnlSDL))
Copy link
Member

Choose a reason for hiding this comment

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

ERROR: clftYeeshaPage08: Error trying to access the Chronicle psnlSDL. psnlSDL = None?

Copy link
Author

Choose a reason for hiding this comment

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

Or at least psnlSDL is falsey. It's like that on master. What do you think should happen here?

else:
PtDebugPrint(u"xKI.GetYeeshaPageDefs(): Trying to access the Vault failed, can't access YeeshaPageChanges Chronicle.", level=kErrorLevel)
PtDebugPrint(u"xKI.GetYeeshaPageDefs(): Trying to access the Chronicle psnlSDL failed: psnlSDL = \"{}\".".format(psnlSDL), level=kErrorLevel)
Copy link
Member

Choose a reason for hiding this comment

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

xKI.GetYeeshaPageDefs(): Trying to access the Chronicle psnlSDL failed: psnlSDL = None?

Copy link
Author

Choose a reason for hiding this comment

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

Or at least psnlSDL is falsey. It's like that on master. What do you think should happen here?

Python/nb01RPSGame.py Outdated Show resolved Hide resolved
Python/nb01RPSGame.py Outdated Show resolved Hide resolved
Python/psnlBahroPoles.py Show resolved Hide resolved
Python/xCheat.py Outdated Show resolved Hide resolved
Python/xMusicBox.py Outdated Show resolved Hide resolved
Python/xPotsSymbol.py Outdated Show resolved Hide resolved
Python/xRandomBoolChange.py Outdated Show resolved Hide resolved
else:
PtDebugPrint("xYeeshaPages: Error trying to access the Vault. Can't access YeeshaPageChanges chronicle." )
PtDebugPrint("xYeeshaPages: Error trying to access the Chronicle psnlSDL. psnlSDL = %s" % ( psnlSDL))
Copy link
Member

Choose a reason for hiding this comment

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

xYeeshaPages: Error trying to access the Chronicle psnlSDL. psnlSDL = None?

Copy link
Author

@ghost ghost Mar 17, 2020

Choose a reason for hiding this comment

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

Or at least psnlSDL is falsey. It's like that on master. What do you think should happen here?

@ghost ghost changed the title replace type(x) == type(y) checks by isinstance replace type(x) == type(y) checks by isinstance and releated Mar 17, 2020
@ghost ghost changed the title replace type(x) == type(y) checks by isinstance and releated replace type(x) == type(y) checks by isinstance and related Mar 17, 2020
@ghost ghost changed the title replace type(x) == type(y) checks by isinstance and related replace type(x) == y checks by isinstance(x, y) and clean up resulting code Mar 17, 2020
@Hoikas Hoikas merged commit 999582f into H-uru:master Mar 26, 2020
@Hoikas
Copy link
Member

Hoikas commented Mar 26, 2020

💥 💥 💥

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.

None yet

1 participant