This is an archive of the discontinued LLVM Phabricator instance.

Differential D55196

Python2/3 compatibility - StringIO
ClosedPublic

Authored by serge-sans-paille on Dec 3 2018, 2:40 AM.

Details

Reviewers
joerg
michaelplatings
Commits
rGf886c03e4709: Portable Python script across Python version
rC350318: Portable Python script across Python version
rL350318: Portable Python script across Python version
Summary

StringIO

Diff Detail

Event Timeline

serge-sans-paille created this revision.Dec 3 2018, 2:40 AM
Herald added a subscriber: arphaman. · View Herald TranscriptDec 3 2018, 2:40 AM
serge-sans-paille retitled this revision from Python2/3 compatibility to Python2/3 compatibility - StringIO.Dec 3 2018, 2:52 AM
michaelplatings added a comment.Dec 3 2018, 3:52 AM

Can you comment on whether the differences between StringIO and io.StringIO might be significant? https://stackoverflow.com/a/49795277

joerg added inline comments.Dec 3 2018, 4:10 AM
bindings/python/tests/cindex/test_translation_unit.py
96

Move it to the import list at the top of the file.

tools/scan-view/share/ScanView.py
110

This and the following cases want byte IO, don't they?

serge-sans-paille updated this revision to Diff 176360.Dec 3 2018, 4:10 AM
StringIO is obsoleted in python3, replaced by io.StringIO.
Yet io.StringIO only handles unicode string, while StringIO.StringIO only handles str, i.e. bytes.
serge-sans-paille marked an inline comment as done.Dec 3 2018, 5:35 AM
serge-sans-paille added inline comments.
tools/scan-view/share/ScanView.py
110

Yeah, it's probably safer. It really depends on the content of `e`. It also appears that Python 2.7 supports io.BytesIO, so I'd rather use that uniformly.

serge-sans-paille updated this revision to Diff 176460.Dec 3 2018, 1:04 PM

Use BytesIO as a repalcement... it more closely matches StringIO.StringIO

serge-sans-paille updated this revision to Diff 176568.Dec 4 2018, 2:12 AM

Update StringIO conversion after more testing.

michaelplatings added inline comments.Dec 13 2018, 2:41 AM
tools/scan-view/share/ScanView.py
115

With Python 2 this fails with:
TypeError: unicode argument expected, got 'str'

michaelplatings requested changes to this revision.Dec 18 2018, 9:32 AM
This revision now requires changes to proceed.Dec 18 2018, 9:32 AM
serge-sans-paille updated this revision to Diff 179059.Dec 20 2018, 6:14 AM
serge-sans-paille updated this revision to Diff 179060.Dec 20 2018, 6:24 AM
michaelplatings requested changes to this revision.Jan 2 2019, 9:50 AM
michaelplatings added inline comments.
bindings/python/tests/cindex/test_translation_unit.py
97

May as well make this >= 3

tools/clang-format/clang-format-diff.py
32

May as well make this >= 3

tools/scan-view/share/ScanView.py
17

May as well make this >= 3

288

Why is this necessary? send_string returns a BytesIO object so why create a different one?

This revision now requires changes to proceed.Jan 2 2019, 9:50 AM
serge-sans-paille updated this revision to Diff 180038.Jan 3 2019, 4:41 AM
serge-sans-paille marked 4 inline comments as done.
michaelplatings accepted this revision.Jan 3 2019, 4:56 AM

LGTM

This revision is now accepted and ready to land.Jan 3 2019, 4:56 AM
Closed by commit rL350318: Portable Python script across Python version (authored by serge_sans_paille). · Explain WhyJan 3 2019, 6:30 AM
This revision was automatically updated to reflect the committed changes.
Herald added a subscriber: llvm-commits. · View Herald TranscriptJan 3 2019, 6:30 AM

Revision Contents

PathSize
bindings/
python/
tests/
cindex/
5 lines
tools/
clang-format/
5 lines
scan-view/
share/
10 lines

Diff 176337

bindings/python/tests/cindex/test_translation_unit.py

Context not available.
self.assertEqual(spellings[-1], 'y') self.assertEqual(spellings[-1], 'y')
def test_unsaved_files_2(self): def test_unsaved_files_2(self):
try: from io import StringIO
joergUnsubmitted
Not Done
ReplyInline Actions

Move it to the import list at the top of the file.

joerg: Move it to the import list at the top of the file.
from StringIO import StringIO
except:
from io import StringIO
tu = TranslationUnit.from_source('fake.c', unsaved_files = [ tu = TranslationUnit.from_source('fake.c', unsaved_files = [
michaelplatingsUnsubmitted
Done
ReplyInline Actions

May as well make this >= 3

michaelplatings: May as well make this >= 3
('fake.c', StringIO('int x;'))]) ('fake.c', StringIO('int x;'))])
spellings = [c.spelling for c in tu.cursor.get_children()] spellings = [c.spelling for c in tu.cursor.get_children()]
Context not available.

tools/clang-format/clang-format-diff.py

Context not available.
import re import re
import subprocess import subprocess
import sys import sys
try: from io import StringIO
from StringIO import StringIO
except ImportError:
from io import StringIO
michaelplatingsUnsubmitted
Done
ReplyInline Actions

May as well make this >= 3

michaelplatings: May as well make this >= 3
def main(): def main():
Context not available.

tools/scan-view/share/ScanView.py

Context not available.
import sys import sys
import urllib, urlparse import urllib, urlparse
import posixpath import posixpath
import StringIO
from io import StringIO
import re import re
import shutil import shutil
import threading import threading
michaelplatingsUnsubmitted
Done
ReplyInline Actions

May as well make this >= 3

michaelplatings: May as well make this >= 3
Context not available.
except Reporter.ReportFailure as e: except Reporter.ReportFailure as e:
self.status = e.value self.status = e.value
except Exception as e: except Exception as e:
s = StringIO.StringIO() s = StringIO()
joergUnsubmitted
Not Done
ReplyInline Actions

This and the following cases want byte IO, don't they?

joerg: This and the following cases want byte IO, don't they?
serge-sans-pailleAuthorUnsubmitted
Done
ReplyInline Actions

Yeah, it's probably safer. It really depends on the content of `e`. It also appears that Python 2.7 supports io.BytesIO, so I'd rather use that uniformly.

serge-sans-paille: Yeah, it's probably safer. It really depends on the content of ``e``. It also appears that…
import traceback import traceback
print >>s,'<b>Unhandled Exception</b><br><pre>' print >>s,'<b>Unhandled Exception</b><br><pre>'
traceback.print_exc(e,file=s) traceback.print_exc(e,file=s)
michaelplatingsUnsubmitted
Not Done
ReplyInline Actions

With Python 2 this fails with:
TypeError: unicode argument expected, got 'str'

michaelplatings: With Python 2 this fails with: TypeError: unicode argument expected, got 'str'
Context not available.
def handle_exception(self, exc): def handle_exception(self, exc):
import traceback import traceback
s = StringIO.StringIO() s = StringIO()
print >>s, "INTERNAL ERROR\n" print >>s, "INTERNAL ERROR\n"
traceback.print_exc(exc, s) traceback.print_exc(exc, s)
f = self.send_string(s.getvalue(), 'text/plain') f = self.send_string(s.getvalue(), 'text/plain')
michaelplatingsUnsubmitted
Done
ReplyInline Actions

Why is this necessary? send_string returns a BytesIO object so why create a different one?

michaelplatings: Why is this necessary? send_string returns a BytesIO object so why create a different one?
Context not available.
mtime = self.dynamic_mtime mtime = self.dynamic_mtime
self.send_header("Last-Modified", self.date_time_string(mtime)) self.send_header("Last-Modified", self.date_time_string(mtime))
self.end_headers() self.end_headers()
return StringIO.StringIO(s) return StringIO(s)
def send_patched_file(self, path, ctype): def send_patched_file(self, path, ctype):
# Allow a very limited set of variables. This is pretty gross. # Allow a very limited set of variables. This is pretty gross.
Context not available.