Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix warnings #140

Merged
merged 4 commits into from
Oct 31, 2017
Merged
Show file tree
Hide file tree
Changes from 2 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
25 changes: 19 additions & 6 deletions smart_open/smart_open_lib.py
Original file line number Diff line number Diff line change
Expand Up @@ -497,19 +497,32 @@ def __exit__(self, type, value, traceback):
pass


def make_closing(base, **attrs):
def make_closing(base, under_stream=None, **attrs):
"""
Add support for `with Base(attrs) as fout:` to the base class if it's missing.
The base class' `close()` method will be called on context exit, to always close the file properly.

This is needed for gzip.GzipFile, bz2.BZ2File etc in older Pythons (<=2.6), which otherwise
raise "AttributeError: GzipFile instance has no attribute '__exit__'".


Some base classes like gzip.GzipFile, bz2.BZ2File etc can wrap the other file-like objects,
but not closing it. It is not a bug, because inner stream are not owned to the base class.
Function make_closing fix that behaviour due to all under_stream-s are created by this library.
"""
if not hasattr(base, '__enter__'):
attrs['__enter__'] = lambda self: self
if not hasattr(base, '__exit__'):
attrs['__exit__'] = lambda self, type, value, traceback: self.close()

__exit__ = getattr(base, '__exit__', None)

def exit_attr(self, type, value, traceback):
if __exit__ is not None:
__exit__(self, type, value, traceback)
else:
self.close()
if under_stream is not None:
under_stream.close()

attrs['__exit__'] = exit_attr
return type('Closing' + base.__name__, (base, object), attrs)


Expand All @@ -530,11 +543,11 @@ def compression_wrapper(file_obj, filename, mode):
from bz2file import BZ2File
else:
from bz2 import BZ2File
return make_closing(BZ2File)(file_obj, mode)
return make_closing(BZ2File, file_obj)(file_obj, mode)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

looks slightly strange file_obj)(file_obj

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Tried redo as just classes


elif ext == '.gz':
from gzip import GzipFile
return make_closing(GzipFile)(fileobj=file_obj, mode=mode)
return make_closing(GzipFile, file_obj)(fileobj=file_obj, mode=mode)

else:
return file_obj
Expand Down
57 changes: 31 additions & 26 deletions smart_open/tests/test_smart_open.py
Original file line number Diff line number Diff line change
Expand Up @@ -122,16 +122,17 @@ def test_http_pass(self):
"""Does http authentication work correctly"""
responses.add(responses.GET, "http://127.0.0.1/index.html", body='line1\nline2')
_ = smart_open.HttpOpenRead(smart_open.ParseUri("http://127.0.0.1/index.html"), user='me', password='pass')
self.assertEquals(len(responses.calls), 1)
self.assertEqual(len(responses.calls), 1)
actual_request = responses.calls[0].request
self.assert_('Authorization' in actual_request.headers)
self.assert_(actual_request.headers['Authorization'].startswith('Basic '))
self.assertTrue('Authorization' in actual_request.headers)
self.assertTrue(actual_request.headers['Authorization'].startswith('Basic '))

@responses.activate
def test_http_gz(self):
"""Can open gzip via http?"""
fpath = os.path.join(CURR_DIR, 'test_data/crlf_at_1k_boundary.warc.gz')
data = open(fpath, 'rb').read()
with open(fpath, 'rb') as file:
data = file.read()

responses.add(responses.GET, "http://127.0.0.1/data.gz",
body=data)
Expand All @@ -146,7 +147,8 @@ def test_http_gz(self):
def test_http_bz2(self):
"""Can open bz2 via http?"""
test_string = b'Hello World Compressed.'
test_file = tempfile.NamedTemporaryFile('wb', suffix='.bz2', delete=False).name
with tempfile.NamedTemporaryFile('wb', suffix='.bz2', delete=False) as file:
test_file = file.name

with smart_open.smart_open(test_file, 'wb') as outfile:
outfile.write(test_string)
Expand Down Expand Up @@ -182,9 +184,9 @@ def test_read_never_returns_none(self):
fout.write(test_string.encode('utf8'))

r = smart_open.smart_open("s3://mybucket/mykey", "rb")
self.assertEquals(r.read(), test_string.encode("utf-8"))
self.assertEquals(r.read(), b"")
self.assertEquals(r.read(), b"")
self.assertEqual(r.read(), test_string.encode("utf-8"))
self.assertEqual(r.read(), b"")
self.assertEqual(r.read(), b"")

@mock_s3
def test_readline(self):
Expand All @@ -196,7 +198,7 @@ def test_readline(self):
fout.write(test_string)

reader = smart_open.smart_open("s3://mybucket/mykey", "rb")
self.assertEquals(reader.readline(), u"hello žluťoučký world!\n".encode("utf-8"))
self.assertEqual(reader.readline(), u"hello žluťoučký world!\n".encode("utf-8"))

@mock_s3
def test_readline_iter(self):
Expand All @@ -210,9 +212,9 @@ def test_readline_iter(self):
reader = smart_open.smart_open("s3://mybucket/mykey", "rb")

actual_lines = [l.decode("utf-8") for l in reader]
self.assertEquals(2, len(actual_lines))
self.assertEquals(lines[0], actual_lines[0])
self.assertEquals(lines[1], actual_lines[1])
self.assertEqual(2, len(actual_lines))
self.assertEqual(lines[0], actual_lines[0])
self.assertEqual(lines[1], actual_lines[1])

@mock_s3
def test_readline_eof(self):
Expand All @@ -224,9 +226,9 @@ def test_readline_eof(self):

reader = smart_open.smart_open("s3://mybucket/mykey", "rb")

self.assertEquals(reader.readline(), b"")
self.assertEquals(reader.readline(), b"")
self.assertEquals(reader.readline(), b"")
self.assertEqual(reader.readline(), b"")
self.assertEqual(reader.readline(), b"")
self.assertEqual(reader.readline(), b"")

@mock_s3
def test_s3_iter_lines(self):
Expand Down Expand Up @@ -785,20 +787,23 @@ def write_read_assertion(self, test_file):
def test_open_gz(self):
"""Can open gzip?"""
fpath = os.path.join(CURR_DIR, 'test_data/crlf_at_1k_boundary.warc.gz')
data = smart_open.smart_open(fpath).read()
with smart_open.smart_open(fpath) as file:
data = file.read()
m = hashlib.md5(data)
assert m.hexdigest() == '18473e60f8c7c98d29d65bf805736a0d', \
'Failed to read gzip'

def test_write_read_gz(self):
"""Can write and read gzip?"""
test_file = tempfile.NamedTemporaryFile('wb', suffix='.gz', delete=False).name
self.write_read_assertion(test_file)
with tempfile.NamedTemporaryFile('wb', suffix='.gz', delete=False) as file:
test_file_name = file.name
self.write_read_assertion(test_file_name)

def test_write_read_bz2(self):
"""Can write and read bz2?"""
test_file = tempfile.NamedTemporaryFile('wb', suffix='.bz2', delete=False).name
self.write_read_assertion(test_file)
with tempfile.NamedTemporaryFile('wb', suffix='.bz2', delete=False) as file:
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

small nitpick - can you rename file (because this is type in python), for example, use infile (here and anywhere)

test_file_name = file.name
self.write_read_assertion(test_file_name)


class MultistreamsBZ2Test(unittest.TestCase):
Expand Down Expand Up @@ -906,11 +911,11 @@ def test_r(self):
key.set_contents_from_string(text.encode("utf-8"))

with smart_open.s3_open_key(key, "r") as fin:
self.assertEquals(fin.read(), u"физкульт-привет!")
self.assertEqual(fin.read(), u"физкульт-привет!")

parsed_uri = smart_open.ParseUri("s3://bucket/key")
with smart_open.s3_open_uri(parsed_uri, "r") as fin:
self.assertEquals(fin.read(), u"физкульт-привет!")
self.assertEqual(fin.read(), u"физкульт-привет!")

def test_bad_mode(self):
"""Bad mode should raise and exception."""
Expand All @@ -930,10 +935,10 @@ def test_rw_encoding(self):
fout.write(text)

with smart_open.s3_open_uri(uri, "r", encoding="koi8-r") as fin:
self.assertEquals(text, fin.read())
self.assertEqual(text, fin.read())

with smart_open.s3_open_uri(uri, "rb") as fin:
self.assertEquals(text.encode("koi8-r"), fin.read())
self.assertEqual(text.encode("koi8-r"), fin.read())

with smart_open.s3_open_uri(uri, "r", encoding="euc-jp") as fin:
self.assertRaises(UnicodeDecodeError, fin.read)
Expand All @@ -958,13 +963,13 @@ def test_rw_gzip(self):
#
with smart_open.s3_open_uri(uri, "rb", ignore_extension=True) as fin:
gz = gzip.GzipFile(fileobj=fin)
self.assertEquals(gz.read().decode("utf-8"), text)
self.assertEqual(gz.read().decode("utf-8"), text)

#
# We should be able to read it back as well.
#
with smart_open.s3_open_uri(uri, "rb") as fin:
self.assertEquals(fin.read().decode("utf-8"), text)
self.assertEqual(fin.read().decode("utf-8"), text)

@mock_s3
def test_gzip_write_mode(self):
Expand Down