Subject: pycodestyle: Use isinstance() for type checking From: Radostin Stoyanov rstoyanov1@gmail.com Wed Oct 11 12:35:41 2017 +0100 Date: Fri Oct 20 11:49:13 2017 -0400: Git: 63fce081ed1e4edf44077546d98b4fcdb3f4884c This is E721 in pycodestyle [1]: "do not compare types, use ‘isinstance()’" The main differece between "type() is" and "isinstance()" is that isinstance() supports inheritance. [1] This can be seen in the example below: >>> type(True) is int False >>> isinstance(True, int) True As we can see in python 'bool' a subclass of 'int'. [1] https://pycodestyle.readthedocs.io/en/latest/intro.html#error-codes [2] https://docs.python.org/2/library/functions.html#isinstance diff --git a/tests/clitest.py b/tests/clitest.py index 7e9e6684..bdc1b448 100644 --- a/tests/clitest.py +++ b/tests/clitest.py @@ -201,7 +201,7 @@ class Command(object): if conn is None: raise RuntimeError("skip check is not None, but conn is None") - if type(check) is str: + if isinstance(check, str): # pylint: disable=protected-access if support._check_version(conn, check): return diff --git a/virtManager/connection.py b/virtManager/connection.py index 70da9220..0f7e20db 100644 --- a/virtManager/connection.py +++ b/virtManager/connection.py @@ -1088,7 +1088,7 @@ class vmmConnection(vmmGObject): try: self._backend.setKeepAlive(20, 1) except Exception as e: - if (type(e) is not AttributeError and + if (not isinstance(e, AttributeError) and not util.is_error_nosupport(e)): raise logging.debug("Connection doesn't support KeepAlive, " diff --git a/virtManager/create.py b/virtManager/create.py index c695b2b2..1fbc65ef 100644 --- a/virtManager/create.py +++ b/virtManager/create.py @@ -1692,7 +1692,7 @@ class vmmCreate(vmmGObjectUI): else: def callback(ignore, text): widget = cbwidget - if type(cbwidget) is str: + if isinstance(cbwidget, str): widget = self.widget(cbwidget) widget.set_text(text) diff --git a/virtManager/details.py b/virtManager/details.py index e74ea5f7..aeac7fd3 100644 --- a/virtManager/details.py +++ b/virtManager/details.py @@ -1520,7 +1520,7 @@ class vmmDetails(vmmGObjectUI): # On Fedora 19, ret is (bool, str) # Someday the bindings might be fixed to just return the str, try # and future proof it a bit - if type(ret) is tuple and len(ret) >= 2: + if isinstance(ret, tuple) and len(ret) >= 2: ret = ret[1] # F24 rawhide, ret[1] is a named tuple with a 'buffer' element... if hasattr(ret, "buffer"): @@ -3218,7 +3218,7 @@ class vmmDetails(vmmGObjectUI): olddev = hw_list_model[i][HW_LIST_COL_DEVICE] # Existing device, don't remove it - if type(olddev) is str or olddev in currentDevices: + if isinstance(olddev, str) or olddev in currentDevices: continue hw_list_model.remove(_iter) diff --git a/virtManager/domain.py b/virtManager/domain.py index 724f83fd..1b278815 100644 --- a/virtManager/domain.py +++ b/virtManager/domain.py @@ -83,7 +83,7 @@ def compare_device(origdev, newdev, idx): if id(origdev) == id(newdev): return True - if type(origdev) is not type(newdev): + if not isinstance(origdev, type(newdev)): return False for devprop in devprops[origdev.virtual_device_type]: diff --git a/virtManager/error.py b/virtManager/error.py index 1de6c8b1..1a8fae5a 100644 --- a/virtManager/error.py +++ b/virtManager/error.py @@ -275,7 +275,7 @@ class vmmErrorDialog(vmmGObject): if _type is not None: pattern = _type name = None - if type(_type) is tuple: + if isinstance(_type, tuple): pattern = _type[0] name = _type[1] diff --git a/virtManager/module_trace.py b/virtManager/module_trace.py index 8d600f4e..ff76504f 100644 --- a/virtManager/module_trace.py +++ b/virtManager/module_trace.py @@ -83,7 +83,7 @@ def wrap_class(classobj): for name in dir(classobj): obj = getattr(classobj, name) - if type(obj) is MethodType: + if isinstance(obj, MethodType): wrap_method(classobj, obj) @@ -92,7 +92,7 @@ def wrap_module(module, regex=None): if regex and not re.match(regex, name): continue obj = getattr(module, name) - if type(obj) is FunctionType: + if isinstance(obj, FunctionType): wrap_func(module, obj) - if type(obj) is ClassType or type(obj) is type: + if isinstance(obj, (ClassType, type)): wrap_class(obj) diff --git a/virtManager/packageutils.py b/virtManager/packageutils.py index 37f96c38..153cc55b 100644 --- a/virtManager/packageutils.py +++ b/virtManager/packageutils.py @@ -38,7 +38,7 @@ def check_packagekit(parent, errbox, packages): if not packages: logging.debug("No PackageKit packages to search for.") return - if type(packages) is not list: + if not isinstance(packages, list): packages = [packages] logging.debug("PackageKit check/install for packages=%s", packages) diff --git a/virtManager/uiutil.py b/virtManager/uiutil.py index 2965c51b..d07c2a1e 100644 --- a/virtManager/uiutil.py +++ b/virtManager/uiutil.py @@ -155,7 +155,7 @@ def set_grid_row_visible(child, visible): based on UI interraction """ parent = child.get_parent() - if type(parent) is not Gtk.Grid: + if not isinstance(parent, Gtk.Grid): raise RuntimeError("Programming error, parent must be grid, " "not %s" % type(parent)) diff --git a/virtManager/viewers.py b/virtManager/viewers.py index 80bdb083..df164f59 100644 --- a/virtManager/viewers.py +++ b/virtManager/viewers.py @@ -614,7 +614,7 @@ class SpiceViewer(Viewer): GObject.GObject.connect(channel, "open-fd", self._channel_open_fd_request) - if (type(channel) == SpiceClientGLib.MainChannel and + if (isinstance(channel, SpiceClientGLib.MainChannel) and not self._main_channel): self._main_channel = channel hid = self._main_channel.connect_after("channel-event", diff --git a/virtconv/ovf.py b/virtconv/ovf.py index 0a770c98..3ba21313 100644 --- a/virtconv/ovf.py +++ b/virtconv/ovf.py @@ -151,7 +151,7 @@ def _import_file(doc, ctx, conn, input_file): ret = ctx.xpathEval(path) result = None if ret is not None: - if type(ret) == list: + if isinstance(ret, list): if len(ret) >= 1: result = ret[0].content else: diff --git a/virtinst/cli.py b/virtinst/cli.py index 17a2e70f..1b86cad5 100644 --- a/virtinst/cli.py +++ b/virtinst/cli.py @@ -1002,7 +1002,7 @@ def _parse_optstr_to_dict(optstr, virtargs, remove_first): virtarg.is_list): optdict[cliname] = [] - if type(optdict.get(cliname)) is list: + if isinstance(optdict.get(cliname), list): optdict[cliname].append(val) else: optdict[cliname] = val @@ -2591,7 +2591,7 @@ class _ParserChar(VirtCLIParser): stub_none = False def support_check(self, inst, virtarg): - if type(virtarg.attrname) is not str: + if not isinstance(virtarg.attrname, str): return if not inst.supports_property(virtarg.attrname): raise ValueError(_("%(devtype)s type '%(chartype)s' does not " diff --git a/virtinst/cloner.py b/virtinst/cloner.py index 72642f60..7345bdad 100644 --- a/virtinst/cloner.py +++ b/virtinst/cloner.py @@ -88,7 +88,7 @@ class Cloner(object): doc="Original guest name.") def set_original_xml(self, val): - if type(val) is not str: + if not isinstance(val, str): raise ValueError(_("Original xml must be a string.")) self._original_xml = val self._original_guest = Guest(self.conn, @@ -214,7 +214,7 @@ class Cloner(object): "(not Cloner.preserve)") def set_force_target(self, dev): - if type(dev) is list: + if isinstance(dev, list): self._force_target = dev[:] else: self._force_target.append(dev) @@ -225,7 +225,7 @@ class Cloner(object): "despite Cloner's recommendation.") def set_skip_target(self, dev): - if type(dev) is list: + if isinstance(dev, list): self._skip_target = dev[:] else: self._skip_target.append(dev) @@ -237,7 +237,7 @@ class Cloner(object): "takes precedence over force_target.") def set_clone_policy(self, policy_list): - if type(policy_list) != list: + if not isinstance(policy_list, list): raise ValueError(_("Cloning policy must be a list of rules.")) self._clone_policy = policy_list def get_clone_policy(self): diff --git a/virtinst/progress.py b/virtinst/progress.py index e9a243b1..d73d4292 100644 --- a/virtinst/progress.py +++ b/virtinst/progress.py @@ -482,7 +482,7 @@ def format_number(number, SI=0, space=' '): depth = depth + 1 number = number / step - if type(number) == type(1) or type(number) == type(long(1)): + if isinstance(number, int) or isinstance(number, long): # it's an int or a long, which means it didn't get divided, # which means it's already short enough format = '%i%s%s' diff --git a/virtinst/urlfetcher.py b/virtinst/urlfetcher.py index 2dc7a726..c57c9e10 100644 --- a/virtinst/urlfetcher.py +++ b/virtinst/urlfetcher.py @@ -1351,7 +1351,7 @@ class ALTLinuxDistro(Distro): def _build_distro_list(): allstores = [] for obj in globals().values(): - if type(obj) is type and issubclass(obj, Distro) and obj.name: + if isinstance(obj, type) and issubclass(obj, Distro) and obj.name: allstores.append(obj) seen_urldistro = [] diff --git a/virtinst/util.py b/virtinst/util.py index 495a0841..2a7bc4d3 100644 --- a/virtinst/util.py +++ b/virtinst/util.py @@ -30,7 +30,7 @@ import libvirt def listify(l): if l is None: return [] - elif type(l) != list: + elif not isinstance(l, list): return [l] else: return l @@ -61,7 +61,7 @@ def libvirt_collision(collision_cb, val): def validate_uuid(val): - if type(val) is not str: + if not isinstance(val, str): raise ValueError(_("UUID must be a string.")) form = re.match("[a-fA-F0-9]{8}[-]([a-fA-F0-9]{4}[-]){3}[a-fA-F0-9]{12}$", @@ -99,7 +99,7 @@ def validate_macaddr(val): if val is None: return - if type(val) is not str: + if not isinstance(val, str): raise ValueError(_("MAC address must be a string.")) form = re.match("^([0-9a-fA-F]{1,2}:){5}[0-9a-fA-F]{1,2}$", val) diff --git a/virtinst/xmlbuilder.py b/virtinst/xmlbuilder.py index 9e234e9f..550fea86 100644 --- a/virtinst/xmlbuilder.py +++ b/virtinst/xmlbuilder.py @@ -886,7 +886,7 @@ class XMLBuilder(object): # XMLChildProperty stores a list in propstore, which dict shallow # copy won't fix for us. for name, value in ret._propstore.items(): - if type(value) is not list: + if not isinstance(value, list): continue ret._propstore[name] = [obj.copy() for obj in ret._propstore[name]]