launchpad-reviewers team mailing list archive

[Colin Watson <cjwatson@xxxxxxxxxxxxx>]

Colin Watson has proposed merging ~cjwatson/launchpad:py3-isinstance-unicode into launchpad:master.

Commit message:
Port isinstance(..., unicode) checks to Python 3

Requested reviews:
  Launchpad code reviewers (launchpad-reviewers)

For more details, see:
https://code.launchpad.net/~cjwatson/launchpad/+git/launchpad/+merge/388947

In most cases, these can just be `isinstance(..., six.text_type)`, but some cases that also check Python 2 `str` (i.e. `bytes`) need slightly different treatment.
-- 
Your team Launchpad code reviewers is requested to review the proposed merge of ~cjwatson/launchpad:py3-isinstance-unicode into launchpad:master.

diff --git a/lib/lp/app/browser/root.py b/lib/lp/app/browser/root.py
index ccfb0fd..0d47b6b 100644
--- a/lib/lp/app/browser/root.py
+++ b/lib/lp/app/browser/root.py
@@ -15,6 +15,7 @@ import time
 import feedparser
 from lazr.batchnavigator.z3batching import batch
 import requests
+import six
 from zope.component import getUtility
 from zope.formlib.interfaces import ConversionError
 from zope.interface import Interface
@@ -427,7 +428,7 @@ class LaunchpadSearchView(LaunchpadFormView):
             if isinstance(error, ConversionError):
                 self.setFieldError(
                     'text', 'Can not convert your search term.')
-            elif isinstance(error, unicode):
+            elif isinstance(error, six.text_type):
                 continue
             elif (error.field_name == 'text'
                 and isinstance(error.errors, TooLong)):
diff --git a/lib/lp/blueprints/browser/specificationgoal.py b/lib/lp/blueprints/browser/specificationgoal.py
index 2458791..003d01d 100644
--- a/lib/lp/blueprints/browser/specificationgoal.py
+++ b/lib/lp/blueprints/browser/specificationgoal.py
@@ -10,6 +10,7 @@ __all__ = [
     ]
 
 
+import six
 from zope.component import getUtility
 
 from lp.blueprints.browser.specificationtarget import HasSpecificationsView
@@ -79,7 +80,7 @@ class GoalDecideView(HasSpecificationsView, LaunchpadView):
                 action = 'Declined'
 
         selected_specs = form['specification']
-        if isinstance(selected_specs, unicode):
+        if isinstance(selected_specs, six.text_type):
             # only a single item was selected, but we want to deal with a
             # list for the general case, so convert it to a list
             selected_specs = [selected_specs]
diff --git a/lib/lp/blueprints/browser/sprint.py b/lib/lp/blueprints/browser/sprint.py
index fabd3e3..aae704a 100644
--- a/lib/lp/blueprints/browser/sprint.py
+++ b/lib/lp/blueprints/browser/sprint.py
@@ -458,7 +458,7 @@ class SprintTopicSetView(HasSpecificationsView, LaunchpadView):
                 action = 'Declined'
 
         selected_specs = form['speclink']
-        if isinstance(selected_specs, unicode):
+        if isinstance(selected_specs, six.text_type):
             # only a single item was selected, but we want to deal with a
             # list for the general case, so convert it to a list
             selected_specs = [selected_specs]
diff --git a/lib/lp/registry/browser/person.py b/lib/lp/registry/browser/person.py
index a8ef6fc..f89f8e0 100644
--- a/lib/lp/registry/browser/person.py
+++ b/lib/lp/registry/browser/person.py
@@ -65,6 +65,7 @@ from lazr.restful.interfaces import IWebServiceClientRequest
 from lazr.restful.utils import smartquote
 from lazr.uri import URI
 import pytz
+import six
 from six.moves.urllib.parse import (
     quote,
     urlencode,
@@ -2773,7 +2774,7 @@ class PersonEditEmailsView(LaunchpadFormView):
         """
         terms = []
         for term in self.unvalidated_addresses:
-            if isinstance(term, unicode):
+            if isinstance(term, six.text_type):
                 term = SimpleTerm(term)
             else:
                 term = SimpleTerm(term, term.email)
@@ -2814,7 +2815,7 @@ class PersonEditEmailsView(LaunchpadFormView):
                 "self.context.id(%s,%d) (%s)"
                 % (person.name, person.id, self.context.name, self.context.id,
                    email.email))
-        elif isinstance(email, unicode):
+        elif isinstance(email, six.text_type):
             tokenset = getUtility(ILoginTokenSet)
             email = tokenset.searchByEmailRequesterAndType(
                 email, self.context, LoginTokenType.VALIDATEEMAIL)
@@ -2940,7 +2941,7 @@ class PersonEditEmailsView(LaunchpadFormView):
         if IEmailAddress.providedBy(emailaddress):
             emailaddress.destroySelf()
             email = emailaddress.email
-        elif isinstance(emailaddress, unicode):
+        elif isinstance(emailaddress, six.text_type):
             logintokenset = getUtility(ILoginTokenSet)
             logintokenset.deleteByEmailRequesterAndType(
                 emailaddress, self.context, LoginTokenType.VALIDATEEMAIL)
diff --git a/lib/lp/registry/interfaces/mailinglist.py b/lib/lp/registry/interfaces/mailinglist.py
index 0924fba..ed735eb 100644
--- a/lib/lp/registry/interfaces/mailinglist.py
+++ b/lib/lp/registry/interfaces/mailinglist.py
@@ -884,7 +884,7 @@ class BaseSubscriptionErrors(Exception):
             non-ascii text (since a person's display name is used here).
         :type error_string: unicode
         """
-        assert isinstance(error_string, unicode), 'Unicode expected'
+        assert isinstance(error_string, six.text_type), 'Unicode expected'
         Exception.__init__(self, error_string)
         self._error_string = error_string
 
diff --git a/lib/lp/registry/model/person.py b/lib/lp/registry/model/person.py
index a496847..5917c6e 100644
--- a/lib/lp/registry/model/person.py
+++ b/lib/lp/registry/model/person.py
@@ -45,6 +45,7 @@ from lazr.restful.utils import (
     smartquote,
     )
 import pytz
+import six
 from sqlobject import (
     BoolCol,
     ForeignKey,
@@ -1296,7 +1297,7 @@ class Person(
             return False
 
         # Translate the team name to an ITeam if we were passed a team.
-        if isinstance(team, (str, unicode)):
+        if isinstance(team, six.string_types):
             team = PersonSet().getByName(team)
             if team is None:
                 # No team, no membership.
@@ -3316,7 +3317,7 @@ class PersonSet:
             "account.")
         db_updated = False
 
-        assert isinstance(openid_identifier, unicode)
+        assert isinstance(openid_identifier, six.text_type)
         assert openid_identifier != u'', (
             "OpenID identifier must not be empty.")
 
diff --git a/lib/lp/services/beautifulsoup.py b/lib/lp/services/beautifulsoup.py
index 5a1862a..c1b5b44 100644
--- a/lib/lp/services/beautifulsoup.py
+++ b/lib/lp/services/beautifulsoup.py
@@ -14,12 +14,14 @@ __all__ = [
 
 from bs4 import BeautifulSoup as _BeautifulSoup
 from bs4.element import SoupStrainer
+import six
 
 
 class BeautifulSoup(_BeautifulSoup):
 
     def __init__(self, markup="", features="html.parser", **kwargs):
-        if not isinstance(markup, unicode) and "from_encoding" not in kwargs:
+        if (not isinstance(markup, six.text_type) and
+                "from_encoding" not in kwargs):
             kwargs["from_encoding"] = "UTF-8"
         super(BeautifulSoup, self).__init__(
             markup=markup, features=features, **kwargs)
diff --git a/lib/lp/services/gpg/handler.py b/lib/lp/services/gpg/handler.py
index 41b7467..e3ac2df 100644
--- a/lib/lp/services/gpg/handler.py
+++ b/lib/lp/services/gpg/handler.py
@@ -23,6 +23,7 @@ import tempfile
 import gpgme
 from lazr.restful.utils import get_current_browser_request
 import requests
+import six
 from six.moves import http_client
 from six.moves.urllib.parse import urlencode
 from zope.interface import implementer
@@ -212,8 +213,8 @@ class GPGHandler:
     def getVerifiedSignature(self, content, signature=None):
         """See IGPGHandler."""
 
-        assert not isinstance(content, unicode)
-        assert not isinstance(signature, unicode)
+        assert not isinstance(content, six.text_type)
+        assert not isinstance(signature, six.text_type)
 
         ctx = get_gpgme_context()
 
@@ -354,7 +355,7 @@ class GPGHandler:
 
     def encryptContent(self, content, key):
         """See IGPGHandler."""
-        if isinstance(content, unicode):
+        if isinstance(content, six.text_type):
             raise TypeError('Content cannot be Unicode.')
 
         ctx = get_gpgme_context()
diff --git a/lib/lp/services/helpers.py b/lib/lp/services/helpers.py
index a23baa8..888c31e 100644
--- a/lib/lp/services/helpers.py
+++ b/lib/lp/services/helpers.py
@@ -18,6 +18,7 @@ import subprocess
 import tarfile
 import warnings
 
+import six
 from zope.security.interfaces import ForbiddenAttribute
 
 
@@ -54,7 +55,7 @@ def text_replaced(text, replacements, _cache={}):
     cachekey = tuple(replacements.items())
     if cachekey not in _cache:
         L = []
-        if isinstance(text, unicode):
+        if isinstance(text, six.text_type):
             list_item = u'(%s)'
             join_char = u'|'
         else:
@@ -350,9 +351,9 @@ def ensure_unicode(string):
     """
     if string is None:
         return None
-    elif isinstance(string, unicode):
+    elif isinstance(string, six.text_type):
         return string
-    elif isinstance(string, basestring):
+    elif isinstance(string, bytes):
         try:
             return string.decode('US-ASCII')
         except UnicodeDecodeError:
diff --git a/lib/lp/services/identity/model/account.py b/lib/lp/services/identity/model/account.py
index b855f66..bfad289 100644
--- a/lib/lp/services/identity/model/account.py
+++ b/lib/lp/services/identity/model/account.py
@@ -11,6 +11,7 @@ __all__ = [
 
 import datetime
 
+import six
 from sqlobject import StringCol
 from storm.locals import ReferenceSet
 from zope.interface import implementer
@@ -102,7 +103,7 @@ class AccountSet:
 
         # Create an OpenIdIdentifier record if requested.
         if openid_identifier is not None:
-            assert isinstance(openid_identifier, unicode)
+            assert isinstance(openid_identifier, six.text_type)
             identifier = OpenIdIdentifier()
             identifier.account = account
             identifier.identifier = openid_identifier
diff --git a/lib/lp/services/mail/notificationrecipientset.py b/lib/lp/services/mail/notificationrecipientset.py
index 5a62fc0..62e9391 100644
--- a/lib/lp/services/mail/notificationrecipientset.py
+++ b/lib/lp/services/mail/notificationrecipientset.py
@@ -12,6 +12,7 @@ __all__ = [
 
 from operator import attrgetter
 
+import six
 from zope.interface import implementer
 from zope.security.proxy import isinstance as zope_isinstance
 
@@ -73,7 +74,7 @@ class NotificationRecipientSet:
 
     def __contains__(self, person_or_email):
         """See `INotificationRecipientSet`."""
-        if zope_isinstance(person_or_email, (str, unicode)):
+        if zope_isinstance(person_or_email, six.string_types):
             return person_or_email in self._emailToPerson
         elif IPerson.providedBy(person_or_email):
             return person_or_email in self._personToRationale
diff --git a/lib/lp/services/mail/sendmail.py b/lib/lp/services/mail/sendmail.py
index b344b12..7cd0739 100644
--- a/lib/lp/services/mail/sendmail.py
+++ b/lib/lp/services/mail/sendmail.py
@@ -46,6 +46,7 @@ from smtplib import SMTP
 import sys
 
 from lazr.restful.utils import get_current_browser_request
+import six
 from zope.component import getUtility
 from zope.security.proxy import (
     isinstance as zisinstance,
@@ -218,7 +219,7 @@ class MailController(object):
     def addAttachment(self, content, content_type='application/octet-stream',
                       inline=False, filename=None, charset=None):
         attachment = Message()
-        if charset and isinstance(content, unicode):
+        if charset and isinstance(content, six.text_type):
             content = content.encode(charset)
         attachment.add_header('Content-Type', content_type)
         if inline:
diff --git a/lib/lp/services/webapp/publisher.py b/lib/lp/services/webapp/publisher.py
index a67601e..fe0316c 100644
--- a/lib/lp/services/webapp/publisher.py
+++ b/lib/lp/services/webapp/publisher.py
@@ -824,7 +824,7 @@ def get_raw_form_value_from_current_request(field, field_name):
     # Zope wrongly encodes any form element that doesn't look like a file,
     # so re-fetch the file content if it has been encoded.
     if request and field_name in request.form and isinstance(
-        request.form[field_name], unicode):
+            request.form[field_name], six.text_type):
         request._environ['wsgi.input'].seek(0)
         fs = FieldStorage(fp=request._body_instream, environ=request._environ)
         return fs[field_name].value
diff --git a/lib/lp/services/webapp/sorting.py b/lib/lp/services/webapp/sorting.py
index 648e4ff..efc4ee2 100644
--- a/lib/lp/services/webapp/sorting.py
+++ b/lib/lp/services/webapp/sorting.py
@@ -10,6 +10,8 @@ __all__ = ['expand_numbers',
 
 import re
 
+import six
+
 
 def expand_numbers(unicode_text, fill_digits=4):
     """Return a copy of the string with numbers zero filled.
@@ -24,7 +26,7 @@ def expand_numbers(unicode_text, fill_digits=4):
     u'branch-0002-0003.0012'
 
     """
-    assert(isinstance(unicode_text, unicode))
+    assert(isinstance(unicode_text, six.text_type))
 
     def substitute_filled_numbers(match):
         return match.group(0).zfill(fill_digits)
@@ -53,8 +55,8 @@ def _reversed_number_comparator(lhs_text, rhs_text):
     -1
 
     """
-    assert isinstance(lhs_text, unicode)
-    assert isinstance(rhs_text, unicode)
+    assert isinstance(lhs_text, six.text_type)
+    assert isinstance(rhs_text, six.text_type)
     translated_lhs_text = lhs_text.translate(reversed_numbers_table)
     translated_rhs_text = rhs_text.translate(reversed_numbers_table)
     return cmp(translated_lhs_text, translated_rhs_text)
diff --git a/lib/lp/soyuz/mail/packageupload.py b/lib/lp/soyuz/mail/packageupload.py
index 5a9e1f0..f194061 100644
--- a/lib/lp/soyuz/mail/packageupload.py
+++ b/lib/lp/soyuz/mail/packageupload.py
@@ -111,7 +111,7 @@ def sanitize_string(s):
         'ascii' codec can't decode byte 0xc4 in position 21: ordinal
         not in range(128)
     """
-    if isinstance(s, unicode):
+    if isinstance(s, six.text_type):
         return s
     else:
         return guess_encoding(s)
diff --git a/lib/lp/testing/factory.py b/lib/lp/testing/factory.py
index 4d93342..431cce1 100644
--- a/lib/lp/testing/factory.py
+++ b/lib/lp/testing/factory.py
@@ -4178,7 +4178,7 @@ class BareLaunchpadObjectFactory(ObjectFactory):
             binpackageformat = BinaryPackageFormat.DEB
         if component is None:
             component = build.source_package_release.component
-        elif isinstance(component, unicode):
+        elif isinstance(component, six.text_type):
             component = getUtility(IComponentSet)[component]
         if isinstance(section_name, basestring):
             section_name = self.makeSection(section_name)
diff --git a/lib/lp/testing/gpgkeys/__init__.py b/lib/lp/testing/gpgkeys/__init__.py
index 53a0845..d330c35 100644
--- a/lib/lp/testing/gpgkeys/__init__.py
+++ b/lib/lp/testing/gpgkeys/__init__.py
@@ -24,6 +24,7 @@ import os
 
 import gpgme
 import scandir
+import six
 from zope.component import getUtility
 
 from lp.registry.interfaces.gpg import IGPGKeySet
@@ -126,10 +127,10 @@ def decrypt_content(content, password):
     :content: encrypted data content
     :password: unicode password to unlock the secret key in question
     """
-    if isinstance(password, unicode):
+    if isinstance(password, six.text_type):
         raise TypeError('Password cannot be Unicode.')
 
-    if isinstance(content, unicode):
+    if isinstance(content, six.text_type):
         raise TypeError('Content cannot be Unicode.')
 
     ctx = get_gpgme_context()
diff --git a/lib/lp/testing/matchers.py b/lib/lp/testing/matchers.py
index fcdba23..c64c461 100644
--- a/lib/lp/testing/matchers.py
+++ b/lib/lp/testing/matchers.py
@@ -472,11 +472,11 @@ class EqualsIgnoringWhitespace(Equals):
     """
 
     def __init__(self, expected):
-        if isinstance(expected, (str, unicode)):
+        if isinstance(expected, six.string_types):
             expected = normalize_whitespace(expected)
         super(EqualsIgnoringWhitespace, self).__init__(expected)
 
     def match(self, observed):
-        if isinstance(observed, (str, unicode)):
+        if isinstance(observed, six.string_types):
             observed = normalize_whitespace(observed)
         return super(EqualsIgnoringWhitespace, self).match(observed)
diff --git a/lib/lp/translations/browser/browser_helpers.py b/lib/lp/translations/browser/browser_helpers.py
index f85cdea..87dbd6d 100644
--- a/lib/lp/translations/browser/browser_helpers.py
+++ b/lib/lp/translations/browser/browser_helpers.py
@@ -17,6 +17,8 @@ __all__ = [
 from math import ceil
 import re
 
+import six
+
 from lp.services import helpers
 from lp.services.webapp.escaping import html_escape
 from lp.translations.interfaces.translations import TranslationConstants
@@ -109,7 +111,7 @@ def convert_newlines_to_web_form(unicode_text):
     if unicode_text is None:
         return None
 
-    assert isinstance(unicode_text, unicode), (
+    assert isinstance(unicode_text, six.text_type), (
         "The given text must be unicode instead of %s" % type(unicode_text))
 
     if unicode_text is None:
diff --git a/lib/lp/translations/utilities/gettext_po_parser.py b/lib/lp/translations/utilities/gettext_po_parser.py
index 27b9385..c11de08 100644
--- a/lib/lp/translations/utilities/gettext_po_parser.py
+++ b/lib/lp/translations/utilities/gettext_po_parser.py
@@ -185,7 +185,7 @@ class POHeader:
         return header_dictionary
 
     def _decode(self, text):
-        if text is None or isinstance(text, unicode):
+        if text is None or isinstance(text, six.text_type):
             # There is noo need to do anything.
             return text
         charset = self.charset
diff --git a/lib/lp/translations/utilities/xpi_header.py b/lib/lp/translations/utilities/xpi_header.py
index 4d5b30b..d2c140e 100644
--- a/lib/lp/translations/utilities/xpi_header.py
+++ b/lib/lp/translations/utilities/xpi_header.py
@@ -11,6 +11,7 @@ from email.utils import parseaddr
 from StringIO import StringIO
 
 import defusedxml.cElementTree as cElementTree
+import six
 from zope.interface import implementer
 
 from lp.translations.interfaces.translationcommonformat import (
@@ -38,15 +39,15 @@ class XpiHeader:
         self.launchpad_export_date = None
         self.comment = None
 
-        if isinstance(header_content, str):
+        if isinstance(header_content, bytes):
             try:
                 self._text = header_content.decode(self.charset)
             except UnicodeDecodeError:
                 raise TranslationFormatInvalidInputError(
                     "XPI header is not encoded in %s." % self.charset)
         else:
-            assert isinstance(header_content, unicode), (
-                "XPI header text is neither str nor unicode.")
+            assert isinstance(header_content, six.text_type), (
+                "XPI header text is neither bytes nor unicode.")
             self._text = header_content
 
     def getRawContent(self):
diff --git a/lib/sqlobject/__init__.py b/lib/sqlobject/__init__.py
index e43407d..1e85316 100644
--- a/lib/sqlobject/__init__.py
+++ b/lib/sqlobject/__init__.py
@@ -8,6 +8,7 @@ __metaclass__ = type
 # SKIP this file when reformatting, due to the sys mangling.
 import datetime
 
+import six
 from storm.exceptions import NotOneError as SQLObjectMoreThanOneResultError
 from storm.expr import SQL
 from storm.sqlobject import *
@@ -39,7 +40,7 @@ def sqlrepr(value, dbname=None):
         return value.getquoted()
     elif isinstance(value, SQL):
         return value.expr
-    elif isinstance(value, (str, unicode)):
+    elif isinstance(value, six.string_types):
         for orig, repl in _sqlStringReplace:
             value = value.replace(orig, repl)
         return "E'%s'" % value