Patchwork D7296: pycompat: kludge around pytype being confused by __new__

login
register
mail settings
Submitter phabricator
Date Nov. 6, 2019, 11:24 p.m.
Message ID <differential-rev-PHID-DREV-jc2oxczrbnmeywjmxqca-req@mercurial-scm.org>
Download mbox | patch
Permalink /patch/42854/
State New
Headers show

Comments

phabricator - Nov. 6, 2019, 11:24 p.m.
durin42 created this revision.
Herald added a subscriber: mercurial-devel.
Herald added a reviewer: hg-reviewers.

REVISION SUMMARY
  We add a function's worth of indirection to construction, but in so
  doing can properly annotate the factory function's types, obviating
  some annoying messes.

REPOSITORY
  rHG Mercurial

REVISION DETAIL
  https://phab.mercurial-scm.org/D7296

AFFECTED FILES
  mercurial/pycompat.py
  mercurial/urllibcompat.py

CHANGE DETAILS




To: durin42, #hg-reviewers
Cc: mercurial-devel
phabricator - Nov. 7, 2019, 8:38 a.m.
indygreg added a comment.


  This one makes me sad because of potential performance regressions due to call overhead. I'd almost rather rip out Python 2 support rather than land this. But I could be convinced if pytypes is as cool as you say...
  
  Is there no way to annotate the types here without the call overhead? Could we do something like an `if False` trick to define a `def bytestr()` with the proper annotations? Surely there's got to be a way...

REPOSITORY
  rHG Mercurial

CHANGES SINCE LAST ACTION
  https://phab.mercurial-scm.org/D7296/new/

REVISION DETAIL
  https://phab.mercurial-scm.org/D7296

To: durin42, #hg-reviewers
Cc: indygreg, mercurial-devel
phabricator - Nov. 7, 2019, 4:07 p.m.
durin42 added a comment.


  I've filed a bug with the pytype team, I'll report back.

REPOSITORY
  rHG Mercurial

CHANGES SINCE LAST ACTION
  https://phab.mercurial-scm.org/D7296/new/

REVISION DETAIL
  https://phab.mercurial-scm.org/D7296

To: durin42, #hg-reviewers
Cc: indygreg, mercurial-devel
phabricator - Nov. 8, 2019, 4:48 p.m.
This revision now requires changes to proceed.
indygreg added a comment.
indygreg requested changes to this revision.


  I'm going to mark for revisions until we know more so this doesn't show up in the reviewable list.

REPOSITORY
  rHG Mercurial

CHANGES SINCE LAST ACTION
  https://phab.mercurial-scm.org/D7296/new/

REVISION DETAIL
  https://phab.mercurial-scm.org/D7296

To: durin42, #hg-reviewers, indygreg
Cc: indygreg, mercurial-devel

Patch

diff --git a/mercurial/urllibcompat.py b/mercurial/urllibcompat.py
--- a/mercurial/urllibcompat.py
+++ b/mercurial/urllibcompat.py
@@ -105,7 +105,7 @@ 
     def quote(s, safe=r'/'):
         # bytestr has an __iter__ that emits characters. quote_from_bytes()
         # does an iteration and expects ints. We coerce to bytes to appease it.
-        if isinstance(s, pycompat.bytestr):
+        if isinstance(s, pycompat._bytestr):
             s = bytes(s)
         s = urllib.parse.quote_from_bytes(s, safe=safe)
         return s.encode('ascii', 'strict')
diff --git a/mercurial/pycompat.py b/mercurial/pycompat.py
--- a/mercurial/pycompat.py
+++ b/mercurial/pycompat.py
@@ -154,7 +154,7 @@ 
     bytechr = struct.Struct(r'>B').pack
     byterepr = b'%r'.__mod__
 
-    class bytestr(bytes):
+    class _bytestr(bytes):
         """A bytes which mostly acts as a Python 2 str
 
         >>> bytestr(), bytestr(bytearray(b'foo')), bytestr(u'ascii'), bytestr(1)
@@ -208,7 +208,7 @@ 
         """
 
         def __new__(cls, s=b''):
-            if isinstance(s, bytestr):
+            if isinstance(s, _bytestr):
                 return s
             if not isinstance(
                 s, (bytes, bytearray)
@@ -398,7 +398,7 @@ 
     unicode = unicode
     bytechr = chr
     byterepr = repr
-    bytestr = str
+    _bytestr = str
     iterbytestr = iter
     maybebytestr = identity
     sysbytes = identity
@@ -504,3 +504,8 @@ 
     return tempfile.NamedTemporaryFile(
         mode, bufsize, suffix=suffix, prefix=prefix, dir=dir, delete=delete
     )
+
+
+def bytestr(s=b''):
+    # type: (Union[bytes,str]) -> _bytestr
+    return _bytestr(s)