Correctly pass newline parameter to built-in open function (piskvorky#478)

* fix(smart_open_lib): add missing newline parameter to _shortcut_open

* fix(smart_open_lib): add missing newline parameter to _shortcut_open

* minor fixes to get tests to pass

* unskip S3 test

* adjust newline unit tests

* update CHANGELOG.md

* fix flake8

* fix: merge conflicts in changelog

Co-authored-by: Andre Burkovski <andre.burkovski@evosoft.com>
Co-authored-by: Michael Penkov <m@penkov.dev>

Author: 3 people

CHANGELOG.md

@@ -1,5 +1,7 @@
 # Unreleased
 
+  - Correctly pass `newline` parameter to built-in `open` function (PR [#478](https://github.com/RaRe-Technologies/smart_open/pull/478), [@burkovae](https://github.com/burkovae))
+
 # 2.0.0, 27 April 2020, "Python 3"
 
   - **This version supports Python 3 only** (3.5+).

smart_open/smart_open_lib.py

@@ -187,6 +187,7 @@ def open(
         buffering=buffering,
         encoding=encoding,
         errors=errors,
+        newline=newline,
     )
     if fobj is not None:
         return fobj
@@ -316,6 +317,7 @@ def _shortcut_open(
         buffering=-1,
         encoding=None,
         errors=None,
+        newline=None,
         ):
     """Try to open the URI using the standard library io.open function.
 
@@ -331,7 +333,6 @@ def _shortcut_open(
 
     :param str uri: A string indicating what to open.
     :param str mode: The mode to pass to the open function.
-    :param dict kw:
     :returns: The opened file
     :rtype: file
     """
@@ -348,10 +349,11 @@ def _shortcut_open(
         return None
 
     open_kwargs = {}
-
     if encoding is not None:
         open_kwargs['encoding'] = encoding
         mode = mode.replace('b', '')
+    if newline is not None:
+        open_kwargs['newline'] = newline
 
     #
     # binary mode of the builtin/stdlib open function doesn't take an errors argument

smart_open/tests/test_smart_open.py

@@ -7,6 +7,7 @@
 #
 
 import bz2
+import csv
 import contextlib
 import io
 import unittest
@@ -911,7 +912,6 @@ def test_s3_read_moto(self):
 
         self.assertEqual(content[14:], smart_open_object.read())  # read the rest
 
-    @unittest.skip('seek functionality for S3 currently disabled because of Issue #152')
     @mock_s3
     def test_s3_seek_moto(self):
         """Does seeking in S3 files work correctly?"""
@@ -1076,6 +1076,29 @@ def test_append_binary_absolute_path(self):
                 mock_open.assert_called_with("/some/file.txt", "wb+", buffering=-1)
                 fout.write(self.as_bytes)
 
+    def test_newline(self):
+        with mock.patch(_BUILTIN_OPEN, mock.Mock(return_value=self.bytesio)) as mock_open:
+            smart_open.smart_open("/some/file.txt", "wb+", newline='\n')
+            mock_open.assert_called_with("/some/file.txt", "wb+", buffering=-1, newline='\n')
+
+    def test_newline_csv(self):
+        #
+        # See https://github.com/RaRe-Technologies/smart_open/issues/477
+        #
+        rows = [{'name': 'alice', 'color': 'aqua'}, {'name': 'bob', 'color': 'blue'}]
+        expected = 'name,color\nalice,aqua\nbob,blue\n'
+
+        with named_temporary_file(mode='w') as tmp:
+            with smart_open.open(tmp.name, 'w+', newline='\n') as fout:
+                out = csv.DictWriter(fout, fieldnames=['name', 'color'])
+                out.writeheader()
+                out.writerows(rows)
+
+            with open(tmp.name, 'r') as fin:
+                content = fin.read()
+
+        assert content == expected
+
     @mock.patch('boto3.Session')
     def test_s3_mode_mock(self, mock_session):
         """Are s3:// open modes passed correctly?"""