Replace StringIO and cStringIO with io.StringIO and io.BytesIO

[Ryan J Ollos]

The ​io module is available since Python 2.6. We can therefore replace StringIO.StringIO and cStringIO.StringIO with io.StringIO and io.BytesIO.

io.StringIO requires a unicode string. io.BytesIO requires a bytes string. StringIO.StringIO allows either unicode or bytes string. cStringIO.StringIO requires a string that is encoded as a bytes string.

[Ryan J Ollos]

Based on what I've read, it would seem that the replacement cStringIO.StringIO → io.BytesIO can be made everywhere. We'll have to be more careful when replacing StringIO.StringIO though.

[Ryan J Ollos]

Proposed changes in log:rjollos.git:t12046_stringio_replacement. I'll do more extensive testing before committing early in milestone:1.3.1. Errors in the proposed changes may point to the need for additional test coverage as well, with the exception of cases that I modified the test case incorrectly. I'm least certain of the changes in the formatter module, and need to test changes to bugzilla2trac.py (unsure whether the attachment content will be utf-8 encoded or unicode). All tests pass on OSX for SQLite, MySQL and PostgreSQL.

[Jun Omae]

Two things:

1. Workflow macro with unicode string raises TypeError: 'unicode' does not have the buffer interface.
2. I think io.BytesIO('...') should be io.BytesIO(b'...')

[Ryan J Ollos]

I've addressed the comment:4 issues in log:rjollos.git:t12046_stringio_replacement.2.

[Jun Omae]

Unit and functional tests pass with the branch on Windows 10 (python 2.7, 32 bit).

However, the following changes in [28fc0d3b/rjollos.git], I think we shouldn't use unicode(wikidom).

@@ -1532,5 +1532,5 @@
         if isinstance(wikidom, basestring):
             wikidom = WikiParser(env).parse(wikidom)
-        self.wikidom = wikidom
+        self.wikidom = unicode(wikidom)
 
     def generate(self, escape_newlines=False):

The wikidom variable would be passed to text argument of Formatter.parse. The text argument accepts a basestring and a iterable instance. See trunk/trac/wiki/formatter.py@14969:1277-1278,1280#L1275​.

Instead, how about the following changes which convert to a unicode if it is a str instance in WikiParser.parse?

trac/wiki/formatter.py

@@ -1531 +1531 @@
         self.context = context
         if isinstance(wikidom, basestring):
             wikidom = WikiParser(env).parse(wikidom)
-        self.wikidom = unicode(wikidom)
+        self.wikidom = wikidom
 
     def generate(self, escape_newlines=False):
         """Generate HTML elements.
@@ -1558 +1558 @@
         self.context = context
         if isinstance(wikidom, basestring):
             wikidom = WikiParser(env).parse(wikidom)
-        self.wikidom = unicode(wikidom)
+        self.wikidom = wikidom
 
     def generate(self, shorten=False):
         """Generate HTML inline elements.

trac/wiki/parser.py

@@ -223 +223 @@
     def parse(self, wikitext):
         """Parse `wikitext` and produce a WikiDOM tree."""
         # obviously still some work to do here ;)
+        if isinstance(wikitext, str):
+            wikitext = wikitext.decode('utf-8')
         return wikitext
 
 

[Ryan J Ollos]

That looks good to me, but I don't know that area of the code very well. Is it possible that wikidom could be an iterable of strings that need to be decoded?

[Jun Omae]

Replying to Ryan J Ollos:

That looks good to me, but I don't know that area of the code very well. Is it possible that wikidom could be an iterable of strings that need to be decoded?

Sounds good.

trac/wiki/formatter.py

@@ -1278 +1278 @@
             text = text.splitlines()
 
         for line in text:
+            if isinstance(line, str):
+                line = line.decode('utf-8')
             # Detect start of code block (new block or embedded block)
             block_start_match = None
             if WikiParser.ENDBLOCK not in line:
@@ -1404 +1406 @@
         processor = None
         buf = io.StringIO()
         for line in text.strip().splitlines():
+            if isinstance(line, str):
+                line = line.decode('utf-8')
             if WikiParser.ENDBLOCK not in line and \
                    WikiParser._startblock_re.match(line):
                 in_code_block += 1
@@ -1531 +1535 @@
         self.context = context
         if isinstance(wikidom, basestring):
             wikidom = WikiParser(env).parse(wikidom)
-        self.wikidom = unicode(wikidom)
+        self.wikidom = wikidom
 
     def generate(self, escape_newlines=False):
         """Generate HTML elements.
@@ -1558 +1562 @@
         self.context = context
         if isinstance(wikidom, basestring):
             wikidom = WikiParser(env).parse(wikidom)
-        self.wikidom = unicode(wikidom)
+        self.wikidom = wikidom
 
     def generate(self, shorten=False):
         """Generate HTML inline elements.

[Ryan J Ollos]

Thanks, committed to trunk in r15010, r15011.

[Ryan J Ollos]

Spotted an error:

trac/mimeview/api.py

@@ -598 +598 @@
         if size == 0:
             return ''
         if self.content is None:
-            self.content = io.StringIO(self.input.read(self.max_size))
+            self.content = io.BytesIO(self.input.read(self.max_size))
         return self.content.read(size)
 
     def reset(self):

I'll add a test case.

[Ryan J Ollos]

Fix with simple test case committed in r15069.

[Ryan J Ollos]

Possible bug in Pygments (using latest, 2.2.0). I tried replacing StringIO.StringIO with io.StringIO and was getting an error with test_python_hello_mimeview​: TypeError: unicode argument expected, got 'str'.

The issue appears to be associated with the newline at the start of the content, and can be reproduced with this minimal test case:

import io

from pygments.formatters.html import HtmlFormatter
from pygments.lexers import get_lexer_by_name


lexer_options = {'stripnl': False}
lexer_name = 'ipython2'

content = """
"""

out = io.StringIO()
lexer = get_lexer_by_name(lexer_name, **lexer_options)
formatter = HtmlFormatter(nowrap=True)
formatter.format(lexer.get_tokens(content), out)

assert '\n' == out.getvalue()

Workaround I've found is to set the HtmlFormatter lineseparator option:

- formatter = HtmlFormatter(nowrap=True)
+ formatter = HtmlFormatter(nowrap=True, lineseparator=u'\n')

The issue is also not seen if adding a single whitespace before the newline. In ​html.py, the code branches to if line is there is a single whitespace and newline, but branches to else: yield 1, lsep if there's only a newline.

[Ryan J Ollos]

We've accumulated some instances of StringIO.StringIO, which will be incompatible with Python 3. Proposed changes in log:rjollos.git:t12046_stringio_replacement.3, including the workaround described in comment:13. I haven't yet tested the deploy_trac.fcgi change.

[Ryan J Ollos]

Replying to Ryan J Ollos:

Possible bug in Pygments (using latest, 2.2.0). I tried replacing StringIO.StringIO with io.StringIO and was getting an error with test_python_hello_mimeview​: TypeError: unicode argument expected, got 'str'.

I've reported the issue to the Pygments project: ​issue 1349.

[Ryan J Ollos]

Replying to Ryan J Ollos:

I haven't yet tested the deploy_trac.fcgi change.

From testing, it seems the type of the traceback is bytes. The change from [15010#file29] was correct but was overwritten in [15424]. Corrected in proposed changes: [01221e2e/rjollos.git].

[Ryan J Ollos]

comment:16 changes committed in r15904.