Opened 7 years ago

Closed 3 years ago

#6289 enhancement closed fixed (fixed)

Port imap4-utf-7 codec implementation to Python 3

Reported by: berdario Owned by: berdario
Priority: normal Milestone: Python-3.x
Component: mail Keywords:
Cc: Thijs Triemstra Branch:
Author:

Description

I started by porting the imap4-utf-7 codec to Python3 Then I ported test_imap.py by looking at all the obvious syntax changes

I did the same with the rest of imap4.py, while keeping it to run correctly with Python2 At then end I fixed the IMAP4UTF7TestCase to run with Python3

So, this patch ended up being a little bit bigger than I expected it to be... when working on it I committed some incremental changes on my bazaar branch, if you want to look at it, it's here: https://code.launchpad.net/~berdario/twisted/imap4-py3

Attachments (1)

imap4.patch (53.3 KB) - added by berdario 7 years ago.

Download all attachments as: .zip

Change History (9)

Changed 7 years ago by berdario

Attachment: imap4.patch added

comment:1 Changed 7 years ago by berdario

3 details I forgot before:

  • the patch also contains a new .bzrignore, you probably don't want it (but since it's part of the bazaar branch, it got included)
  • my commit 16492 message's "now the tests pass" doesn't really mean anything: since the module wasn't added to _twistedpython3.py it was passing 0 out 0 tests (so yes: technically it was passing all of them :) )

comment:2 Changed 7 years ago by Itamar Turner-Trauring

Keywords: review python3 removed
Milestone: Python-3.x
Owner: set to berdario
  1. It's not clear from the description if this is actually a full port of the module to Python 3. Is it? Besides anything you know you haven't ported, if you run the tests under coverage.py with branch coverage, are parts of the code not tested?
  2. imap module should be added to ported modules list (if it's ported), and cred modules to list of unported but necessary modules (since you're just getting them to import).
  3. If you're going to expand the public API of compat you need to add intTypes to __all__ and also document it (in this case in the module docstring). And have a test, for that matter. At which point that probably should be a separate ticket.

(I'm tempted to suggest the IMAP code also be split into multiple tickets, but want to hear the answer to the first question.)

comment:3 Changed 7 years ago by Thijs Triemstra

Cc: Thijs Triemstra added

comment:4 Changed 7 years ago by berdario

  1. No, it's not a full port... I run coverage.py and indeed there're 3 lines in the encoding/decoding functions that aren't tested: 6177, 6178, 6217
  2. the imap module has already been added to the ported modules (but it's not entirely ported)... I need to add cred to the almostModules
  3. yes, I forgot about compat.all and the other things

comment:5 Changed 7 years ago by Jean-Paul Calderone

Summary: Imap4 changes for Python3 compatibilityPort imap4-utf-7 codec implementation to Python 3

comment:6 Changed 3 years ago by berdario

Owner: berdario deleted

After 4 years, I see that now Twisted has some Python3 compatibility (I'm actually using it... but upgrading is a pain, since support is still unstable) and some of the changes I did have been applied already.

This made it simple to recreate the changes to make IMAP4UTF7TestCase pass, without changing too much code

https://github.com/twisted/twisted/pull/541

I tested my changes on both Python3.5 and Python2.7 by running

py.test test_imap.py -k UTF7

comment:8 Changed 3 years ago by Craig Rodrigues <rodrigc@…>

Resolution: fixed
Status: newclosed

In 9fd621c:

Merge pull request #541 from berdario/py3-imap4-take2

Author: berdario
Reviewer: rodrigc
Fixes: #6289

Port imap4-utf-7 codec implementation to Python 3

Note: See TracTickets for help on using tickets.