Patchwork D2752: cbor: add a __init__.py to top level cbor module

login
register
mail settings
Submitter phabricator
Date March 9, 2018, 10:54 a.m.
Message ID <differential-rev-PHID-DREV-sjybklz2h2dnvx6of4o2-req@phab.mercurial-scm.org>
Download mbox | patch
Permalink /patch/29157/
State Superseded
Headers show

Comments

phabricator - March 9, 2018, 10:54 a.m.
pulkit created this revision.
Herald added a subscriber: mercurial-devel.
Herald added a reviewer: hg-reviewers.

REVISION SUMMARY
  This patch also fixes import in cbor2/ to make them relative.

REPOSITORY
  rHG Mercurial

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

AFFECTED FILES
  mercurial/thirdparty/cbor/__init__.py
  mercurial/thirdparty/cbor/cbor2/__init__.py
  mercurial/thirdparty/cbor/cbor2/decoder.py
  mercurial/thirdparty/cbor/cbor2/encoder.py

CHANGE DETAILS




To: pulkit, #hg-reviewers
Cc: mercurial-devel
phabricator - March 9, 2018, 4:19 p.m.
indygreg added a comment.


  I think you should send the relative import patches to upstream. Adding `from __future__ import absolute_import` would also be a nice touch.

REPOSITORY
  rHG Mercurial

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

To: pulkit, #hg-reviewers
Cc: indygreg, mercurial-devel
phabricator - March 14, 2018, 9:02 a.m.
pulkit added a comment.


  In https://phab.mercurial-scm.org/D2752#44289, @indygreg wrote:
  
  > I think you should send the relative import patches to upstream. Adding `from __future__ import absolute_import` would also be a nice touch.
  
  
  I tried that at https://github.com/agronholm/cbor2/pull/20.  I might be doing something wrong here but I am not sure what's that.

REPOSITORY
  rHG Mercurial

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

To: pulkit, #hg-reviewers
Cc: indygreg, mercurial-devel
phabricator - March 22, 2018, 3:28 p.m.
indygreg added a subscriber: alex_gaynor.
indygreg added a comment.


  In https://phab.mercurial-scm.org/D2752#45926, @pulkit wrote:
  
  > In https://phab.mercurial-scm.org/D2752#44289, @indygreg wrote:
  >
  > > I think you should send the relative import patches to upstream. Adding `from __future__ import absolute_import` would also be a nice touch.
  >
  >
  > I tried that at https://github.com/agronholm/cbor2/pull/20.  I might be doing something wrong here but I am not sure what's that.
  
  
  I started engaging the package author on that PR. I care enough about this issue that I may start a discussion on a Python mailing list about it. Not sure which one though.
  
  @alex_gaynor: if I wanted to convince some heavy hitters in the Python community that relative imports within packages should be a best practice, how would you recommend going about that?

REPOSITORY
  rHG Mercurial

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

To: pulkit, #hg-reviewers
Cc: alex_gaynor, indygreg, mercurial-devel
phabricator - March 22, 2018, 3:32 p.m.
alex_gaynor added a comment.


  In https://phab.mercurial-scm.org/D2752#47299, @indygreg wrote:
  
  > @alex_gaynor: if I wanted to convince some heavy hitters in the Python community that relative imports within packages should be a best practice, how would you recommend going about that?
  
  
  The heaviest stick would be to write a PEP encouraging this practice, that way there'd be a canonical place to point people.
  
  An intermediate step might be starting a thread on python-dev (or maybe python-ideas) about it.
  
  You can also always blog about why it's useful. I think best practices around relative imports (explicit and implicit) were formed in a different era and deserve reconsideration.

REPOSITORY
  rHG Mercurial

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

To: pulkit, #hg-reviewers
Cc: alex_gaynor, indygreg, mercurial-devel
phabricator - March 23, 2018, 6:26 p.m.
indygreg added a comment.


  In https://phab.mercurial-scm.org/D2752#47301, @alex_gaynor wrote:
  
  > In https://phab.mercurial-scm.org/D2752#47299, @indygreg wrote:
  >
  > >
  >
  >
  > An intermediate step might be starting a thread on python-dev (or maybe python-ideas) about it.
  
  
  https://mail.python.org/pipermail/python-dev/2018-March/152446.html

REPOSITORY
  rHG Mercurial

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

To: pulkit, #hg-reviewers
Cc: alex_gaynor, indygreg, mercurial-devel
phabricator - March 24, 2018, 4:01 p.m.
martinvonz added a comment.


  In https://phab.mercurial-scm.org/D2752#45926, @pulkit wrote:
  
  > In https://phab.mercurial-scm.org/D2752#44289, @indygreg wrote:
  >
  > > I think you should send the relative import patches to upstream. Adding `from __future__ import absolute_import` would also be a nice touch.
  >
  >
  > I tried that at https://github.com/agronholm/cbor2/pull/20.  I might be doing something wrong here but I am not sure what's that.
  
  
  The relative imports are now done: https://github.com/agronholm/cbor2/commit/84181540f6eb650437e3f73cd104a65661fe8e67

REPOSITORY
  rHG Mercurial

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

To: pulkit, #hg-reviewers
Cc: martinvonz, alex_gaynor, indygreg, mercurial-devel
phabricator - March 26, 2018, 3:34 p.m.
indygreg accepted this revision.
indygreg added a comment.
This revision is now accepted and ready to land.


  I vendored upstream commit 84181540f6eb650437e3f73cd104a65661fe8e67. So the relative imports will drop from this commit when it is pushed.

REPOSITORY
  rHG Mercurial

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

To: pulkit, #hg-reviewers, indygreg
Cc: martinvonz, alex_gaynor, indygreg, mercurial-devel
phabricator - March 26, 2018, 3:43 p.m.
indygreg added a comment.


  I also fixed this in flight to make the import in `__init__.py` relative.

REPOSITORY
  rHG Mercurial

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

To: pulkit, #hg-reviewers, indygreg
Cc: martinvonz, alex_gaynor, indygreg, mercurial-devel

Patch

diff --git a/mercurial/thirdparty/cbor/cbor2/encoder.py b/mercurial/thirdparty/cbor/cbor2/encoder.py
--- a/mercurial/thirdparty/cbor/cbor2/encoder.py
+++ b/mercurial/thirdparty/cbor/cbor2/encoder.py
@@ -6,9 +6,9 @@ 
 from datetime import datetime, date, time
 from io import BytesIO
 
-from cbor2.compat import iteritems, timezone, long, unicode, as_unicode, bytes_from_list
-from cbor2.compat import pack_float16, unpack_float16
-from cbor2.types import CBORTag, undefined, CBORSimpleValue
+from .compat import iteritems, timezone, long, unicode, as_unicode, bytes_from_list
+from .compat import pack_float16, unpack_float16
+from .types import CBORTag, undefined, CBORSimpleValue
 
 
 class CBOREncodeError(Exception):
diff --git a/mercurial/thirdparty/cbor/cbor2/decoder.py b/mercurial/thirdparty/cbor/cbor2/decoder.py
--- a/mercurial/thirdparty/cbor/cbor2/decoder.py
+++ b/mercurial/thirdparty/cbor/cbor2/decoder.py
@@ -3,8 +3,8 @@ 
 from datetime import datetime, timedelta
 from io import BytesIO
 
-from cbor2.compat import timezone, xrange, byte_as_integer, unpack_float16
-from cbor2.types import CBORTag, undefined, break_marker, CBORSimpleValue
+from .compat import timezone, xrange, byte_as_integer, unpack_float16
+from .types import CBORTag, undefined, break_marker, CBORSimpleValue
 
 timestamp_re = re.compile(r'^(\d{4})-(\d\d)-(\d\d)T(\d\d):(\d\d):(\d\d)'
                           r'(?:\.(\d+))?(?:Z|([+-]\d\d):(\d\d))$')
diff --git a/mercurial/thirdparty/cbor/cbor2/__init__.py b/mercurial/thirdparty/cbor/cbor2/__init__.py
--- a/mercurial/thirdparty/cbor/cbor2/__init__.py
+++ b/mercurial/thirdparty/cbor/cbor2/__init__.py
@@ -1,3 +1,3 @@ 
-from cbor2.decoder import load, loads, CBORDecoder, CBORDecodeError  # noqa
-from cbor2.encoder import dump, dumps, CBOREncoder, CBOREncodeError, shareable_encoder  # noqa
-from cbor2.types import CBORTag, CBORSimpleValue, undefined  # noqa
+from .decoder import load, loads, CBORDecoder, CBORDecodeError  # noqa
+from .encoder import dump, dumps, CBOREncoder, CBOREncodeError, shareable_encoder  # noqa
+from .types import CBORTag, CBORSimpleValue, undefined  # noqa
diff --git a/mercurial/thirdparty/cbor/__init__.py b/mercurial/thirdparty/cbor/__init__.py
new file mode 100644
--- /dev/null
+++ b/mercurial/thirdparty/cbor/__init__.py
@@ -0,0 +1 @@ 
+from cbor2 import load, dump, CBORDecodeError, CBOREncodeError