From 9913ecdee3664f43a401c5a507ce14d6eefa0af2 Mon Sep 17 00:00:00 2001 From: John Chilton Date: Sun, 12 May 2019 21:49:14 -0400 Subject: [PATCH 01/14] Fix extra newlines in peek code. Behavior changes: - Now includes a trailing new line if the input contained a trailing newline. - Now returns only as many lines as are present if the file contains fewer lines than LINE_COUNT. --- lib/galaxy/datatypes/data.py | 35 ++++++++++++++++-------- lib/galaxy/datatypes/test/0_nonewline | 1 + test/integration/test_datatype_upload.py | 2 +- test/unit/datatypes/test_data.py | 2 +- 4 files changed, 27 insertions(+), 13 deletions(-) create mode 100644 lib/galaxy/datatypes/test/0_nonewline diff --git a/lib/galaxy/datatypes/data.py b/lib/galaxy/datatypes/data.py index c0feebd7d52..c0ba39e006b 100644 --- a/lib/galaxy/datatypes/data.py +++ b/lib/galaxy/datatypes/data.py @@ -1030,8 +1030,14 @@ def get_file_peek(file_name, is_multi_byte=False, WIDTH=256, LINE_COUNT=5, skipc :param is_multi_byte: deprecated :type is_multi_byte: bool - >>> fname = get_test_fname('4.bed') - >>> assert get_file_peek(fname, LINE_COUNT=1) == u'chr22\\t30128507\\t31828507\\tuc003bnx.1_cds_2_0_chr22_29227_f\\t0\\t+\\n' + >>> def assert_peek_is(file_name, expected, *args, **kwd): + ... path = get_test_fname(file_name) + ... peek = get_file_peek(path, *args, **kwd) + ... assert peek == expected, "%s != %s" % (peek, expected) + >>> assert_peek_is('0_nonewline', u'0') + >>> assert_peek_is('0.txt', u'0\\n') + >>> assert_peek_is('4.bed', u'chr22\\t30128507\\t31828507\\tuc003bnx.1_cds_2_0_chr22_29227_f\\t0\\t+\\n', LINE_COUNT=1) + >>> assert_peek_is('1.bed', u'chr1\\t147962192\\t147962580\\tCCDS989.1_cds_0_0_chr1_147962193_r\\t0\\t-\\nchr1\\t147984545\\t147984630\\tCCDS990.1_cds_0_0_chr1_147984546_f\\t0\\t+\\n', LINE_COUNT=2) """ # Set size for file.readline() to a negative number to force it to # read until either a newline or EOF. Needed for datasets with very @@ -1042,20 +1048,27 @@ def get_file_peek(file_name, is_multi_byte=False, WIDTH=256, LINE_COUNT=5, skipc skipchars = [] lines = [] count = 0 + + last_line_break = False with compression_utils.get_fileobj(file_name, "U") as temp: while count < LINE_COUNT: try: line = temp.readline(WIDTH) except UnicodeDecodeError: return "binary file" - if not line_wrap: - if line.endswith('\n'): - line = line[:-1] - else: - while True: - i = temp.read(1) - if not i or i == '\n': - break + if line == "": + break + last_line_break = False + if line.endswith('\n'): + line = line[:-1] + last_line_break = True + elif not line_wrap: + while True: + i = temp.read(1) + if i == '\n': + last_line_break = True + if not i or i == '\n': + break skip_line = False for skipchar in skipchars: if line.startswith(skipchar): @@ -1064,4 +1077,4 @@ def get_file_peek(file_name, is_multi_byte=False, WIDTH=256, LINE_COUNT=5, skipc if not skip_line: lines.append(line) count += 1 - return '\n'.join(lines) + return '\n'.join(lines) + ('\n' if last_line_break else '') diff --git a/lib/galaxy/datatypes/test/0_nonewline b/lib/galaxy/datatypes/test/0_nonewline new file mode 100644 index 00000000000..c227083464f --- /dev/null +++ b/lib/galaxy/datatypes/test/0_nonewline @@ -0,0 +1 @@ +0 \ No newline at end of file diff --git a/test/integration/test_datatype_upload.py b/test/integration/test_datatype_upload.py index 4c7c742c534..58e45b0bb55 100644 --- a/test/integration/test_datatype_upload.py +++ b/test/integration/test_datatype_upload.py @@ -32,7 +32,7 @@ def find_datatype(registry, filename): def collect_test_data(registry): - test_files = os.listdir(TEST_FILE_DIR) + test_files = [f for f in os.listdir(TEST_FILE_DIR) if "." in f] files = [os.path.join(TEST_FILE_DIR, f) for f in test_files] datatypes = [find_datatype(registry, f) for f in test_files] uploadable = [datatype.file_ext in registry.upload_file_formats for datatype in datatypes] diff --git a/test/unit/datatypes/test_data.py b/test/unit/datatypes/test_data.py index 969916d57b9..d1f85571714 100644 --- a/test/unit/datatypes/test_data.py +++ b/test/unit/datatypes/test_data.py @@ -10,4 +10,4 @@ from galaxy.util import galaxy_directory def test_get_file_peek(): # should get the first 5 lines of the file without a trailing newline character - assert get_file_peek(os.path.join(galaxy_directory(), 'test-data/1.tabular'), line_wrap=False) == 'chr22\t1000\tNM_17\nchr22\t2000\tNM_18\nchr10\t2200\tNM_10\nchr10\thap\ttest\nchr10\t1200\tNM_11' + assert get_file_peek(os.path.join(galaxy_directory(), 'test-data/1.tabular'), line_wrap=False) == 'chr22\t1000\tNM_17\nchr22\t2000\tNM_18\nchr10\t2200\tNM_10\nchr10\thap\ttest\nchr10\t1200\tNM_11\n' From 8ae1dd1e4dc5c849a299e7ce78584ca0f67740ad Mon Sep 17 00:00:00 2001 From: John Chilton Date: Tue, 14 May 2019 15:13:01 -0400 Subject: [PATCH 02/14] Improved newline conversion handling. - Fix bug where sep2tabs didn't work with '\r\n' - Fix bug where sep2tabs didn't work with '\r' - we were opening it with universal newlines. - Fix memory bug where convert_newlines would read unbounded buffers - other methods still do though :(. - Simplify convert_newlines to just let universal newline handling handle the conversion. --- lib/galaxy/datatypes/sniff.py | 90 +++++++++++++++++++++---------- test/unit/datatypes/test_sniff.py | 41 ++++++++++++++ 2 files changed, 104 insertions(+), 27 deletions(-) create mode 100644 test/unit/datatypes/test_sniff.py diff --git a/lib/galaxy/datatypes/sniff.py b/lib/galaxy/datatypes/sniff.py index 404f9c53a11..d01fd7bb55d 100644 --- a/lib/galaxy/datatypes/sniff.py +++ b/lib/galaxy/datatypes/sniff.py @@ -103,28 +103,63 @@ def stream_to_file(stream, suffix='', prefix='', dir=None, text=False, **kwd): return stream_to_open_named_file(stream, fd, temp_name, **kwd) -def convert_newlines(fname, in_place=True, tmp_dir=None, tmp_prefix="gxupload"): +def convert_newlines(fname, in_place=True, tmp_dir=None, tmp_prefix="gxupload", block_size=128 * 1024): """ Converts in place a file from universal line endings to Posix line endings. - >>> fname = get_test_fname('temp.txt') - >>> with open(fname, 'wt') as fh: - ... _ = fh.write("1 2\\r3 4") - >>> convert_newlines(fname, tmp_prefix="gxtest", tmp_dir=tempfile.gettempdir()) - (2, None) - >>> open(fname).read() - '1 2\\n3 4\\n' + >>> def assert_converts_to_1234(content, block_size=1024): + ... fname = get_test_fname('temp.txt') + ... with open(fname, 'w') as fh: + ... _ = fh.write(content) + ... rval = convert_newlines(fname, tmp_prefix="gxtest", tmp_dir=tempfile.gettempdir(), block_size=block_size) + ... assert rval == (2, None), rval + ... actual_contents = open(fname).read() + ... assert '1 2\\n3 4\\n' == actual_contents, actual_contents + >>> # Verify ends with newline - with or without that on inputs - for any of + >>> # \\r \\n or \\r\\n newlines. + >>> assert_converts_to_1234("1 2\\r3 4") + >>> assert_converts_to_1234("1 2\\n3 4") + >>> assert_converts_to_1234("1 2\\r\\n3 4") + >>> assert_converts_to_1234("1 2\\r3 4\\r") + >>> assert_converts_to_1234("1 2\\n3 4\\n") + >>> assert_converts_to_1234("1 2\\r\\n3 4\\r\\n") + >>> assert_converts_to_1234("1 2\\r3 4", block_size=2) + >>> assert_converts_to_1234("1 2\\n3 4", block_size=2) + >>> assert_converts_to_1234("1 2\\r\\n3 4", block_size=2) + >>> assert_converts_to_1234("1 2\\r3 4\\r", block_size=2) + >>> assert_converts_to_1234("1 2\\n3 4\\n", block_size=2) + >>> assert_converts_to_1234("1 2\\r\\n3 4\\r\\n", block_size=2) + >>> assert_converts_to_1234("1 2\\r3 4", block_size=3) + >>> assert_converts_to_1234("1 2\\n3 4", block_size=3) + >>> assert_converts_to_1234("1 2\\r\\n3 4", block_size=3) + >>> assert_converts_to_1234("1 2\\r3 4\\r", block_size=3) + >>> assert_converts_to_1234("1 2\\n3 4\\n", block_size=3) + >>> assert_converts_to_1234("1 2\\r\\n3 4\\r\\n", block_size=3) """ fd, temp_name = tempfile.mkstemp(prefix=tmp_prefix, dir=tmp_dir) + i = 0 with io.open(fd, mode="wt", encoding='utf-8') as fp: - i = None - for i, line in enumerate(io.open(fname, encoding='utf-8')): - fp.write("%s\n" % line.rstrip("\r\n")) - if i is None: - i = 0 - else: - i += 1 + with io.open(fname, encoding='utf-8') as fi: + partial_line = False + while True: + line = fi.readline(block_size) + if not line: + if partial_line: + fp.write(u"\n") + i += 1 + break + + if line[-1] == u"\n": + partial_line = False + fp.write(line) + i += 1 + continue + + # We have a block... maybe at the end of the file. + partial_line = True + fp.write(line) + if in_place: shutil.move(temp_name, fname) # Return number of lines in file. @@ -136,24 +171,20 @@ def convert_newlines(fname, in_place=True, tmp_dir=None, tmp_prefix="gxupload"): def sep2tabs(fname, in_place=True, patt=r"\s+", tmp_dir=None, tmp_prefix="gxupload"): """ Transforms in place a 'sep' separated file to a tab separated one - - >>> fname = get_test_fname('temp.txt') - >>> with open(fname, 'wt') as fh: - ... _ = fh.write(u"1 2\\n3 4\\n") - >>> sep2tabs(fname) - (2, None) - >>> open(fname).read() - '1\\t2\\n3\\t4\\n' """ regexp = re.compile(patt) fd, temp_name = tempfile.mkstemp(prefix=tmp_prefix, dir=tmp_dir) - with io.open(fd, mode="wt", encoding='utf-8') as fp: + with io.open(fd, mode="w", encoding='utf-8') as fp: i = None - for i, line in enumerate(io.open(fname, encoding='utf-8')): + for i, line in enumerate(io.open(fname, encoding='utf-8', newline='')): if line.endswith("\r"): line = line.rstrip('\r') elems = regexp.split(line) fp.write(u"%s\r" % '\t'.join(elems)) + elif line.endswith("\r\n"): + line = line.rstrip('\r\n') + elems = regexp.split(line) + fp.write(u"%s\r\n" % '\t'.join(elems)) else: line = line.rstrip('\n') elems = regexp.split(line) @@ -186,16 +217,21 @@ def convert_newlines_sep2tabs(fname, in_place=True, patt=r"\s+", tmp_dir=None, t regexp = re.compile(patt) fd, temp_name = tempfile.mkstemp(prefix=tmp_prefix, dir=tmp_dir) with io.open(fd, mode="wt", encoding='utf-8') as fp: + i = None for i, line in enumerate(io.open(fname, encoding='utf-8')): line = line.rstrip('\r\n') elems = regexp.split(line) fp.write(u"%s\n" % '\t'.join(elems)) + if i is None: + i = 0 + else: + i = i + 1 if in_place: shutil.move(temp_name, fname) # Return number of lines in file. - return (i + 1, None) + return (i, None) else: - return (i + 1, temp_name) + return (i, temp_name) def iter_headers(fname_or_file_prefix, sep, count=60, comment_designator=None): diff --git a/test/unit/datatypes/test_sniff.py b/test/unit/datatypes/test_sniff.py new file mode 100644 index 00000000000..a41f891dba3 --- /dev/null +++ b/test/unit/datatypes/test_sniff.py @@ -0,0 +1,41 @@ +import tempfile + +from galaxy.datatypes.sniff import convert_newlines_sep2tabs, sep2tabs + + +def assert_converts_to_1234_sep2tabs(content, line_ending="\n"): + print("\r" in content) + tf = tempfile.NamedTemporaryFile(delete=False, mode='w') + tf.write(content) + tf.close() + rval = sep2tabs(tf.name, tmp_prefix="gxtest", tmp_dir=tempfile.gettempdir()) + assert rval == (2, None), rval + assert '1\t2%s3\t4%s' % (line_ending, line_ending) == open(tf.name).read() + + +def assert_converts_to_1234_convert_sep2tabs(content, expected='1\t2\n3\t4\n', line_ending="\n"): + print("\r" in content) + tf = tempfile.NamedTemporaryFile(delete=False, mode='w') + tf.write(content) + tf.close() + rval = convert_newlines_sep2tabs(tf.name, tmp_prefix="gxtest", tmp_dir=tempfile.gettempdir()) + assert rval == (2, None), rval + assert expected == open(tf.name).read() + + +def test_sep2tabs(): + assert_converts_to_1234_sep2tabs("1 2\n3 4\n") + assert_converts_to_1234_sep2tabs("1 2\n3 4\n") + assert_converts_to_1234_sep2tabs("1\t2\n3\t4\n") + assert_converts_to_1234_sep2tabs("1\t2\r3\t4\r", line_ending='\r') + assert_converts_to_1234_sep2tabs("1\t2\r\n3\t4\r\n", line_ending='\r\n') + + +def test_convert_sep2tabs(): + assert_converts_to_1234_convert_sep2tabs("1 2\n3 4\n") + assert_converts_to_1234_convert_sep2tabs("1 2\n3 4\n") + assert_converts_to_1234_convert_sep2tabs("1\t2\n3\t4\n") + assert_converts_to_1234_convert_sep2tabs("1\t2\r3\t4\r") + assert_converts_to_1234_convert_sep2tabs("1\t2\r\n3\t4\r\n") + assert_converts_to_1234_convert_sep2tabs("1 2\r\n3 4\r\n") + assert_converts_to_1234_convert_sep2tabs("1 2 \n3 4 \n", expected='1\t2\t\n3\t4\t\n') From fc5d06804488ce7afcd4e326d20eb2f26b61e6ef Mon Sep 17 00:00:00 2001 From: John Chilton Date: Wed, 15 May 2019 14:49:59 -0400 Subject: [PATCH 03/14] Attempt binary conversion of newlines if UTF conversion fails. --- lib/galaxy/datatypes/sniff.py | 58 +++-- lib/galaxy/datatypes/test/1.imzml | 380 +++++++++++++++++++++++++++++ lib/galaxy/datatypes/test/dosimzml | 380 +++++++++++++++++++++++++++++ test/unit/datatypes/test_sniff.py | 14 +- 4 files changed, 815 insertions(+), 17 deletions(-) create mode 100644 lib/galaxy/datatypes/test/1.imzml create mode 100644 lib/galaxy/datatypes/test/dosimzml diff --git a/lib/galaxy/datatypes/sniff.py b/lib/galaxy/datatypes/sniff.py index d01fd7bb55d..a629fc15695 100644 --- a/lib/galaxy/datatypes/sniff.py +++ b/lib/galaxy/datatypes/sniff.py @@ -139,26 +139,52 @@ def convert_newlines(fname, in_place=True, tmp_dir=None, tmp_prefix="gxupload", """ fd, temp_name = tempfile.mkstemp(prefix=tmp_prefix, dir=tmp_dir) i = 0 - with io.open(fd, mode="wt", encoding='utf-8') as fp: - with io.open(fname, encoding='utf-8') as fi: - partial_line = False - while True: - line = fi.readline(block_size) - if not line: - if partial_line: - fp.write(u"\n") + try: + with io.open(fd, mode="wt", encoding='utf-8') as fp: + with io.open(fname, encoding='utf-8') as fi: + partial_line = False + while True: + line = fi.readline(block_size) + if not line: + if partial_line: + fp.write(u"\n") + i += 1 + break + + if line[-1] == u"\n": + partial_line = False + fp.write(line) i += 1 - break + continue - if line[-1] == u"\n": - partial_line = False + # We have a block... maybe at the end of the file. + partial_line = True fp.write(line) - i += 1 - continue + except UnicodeDecodeError: + fd, temp_name = tempfile.mkstemp(prefix=tmp_prefix, dir=tmp_dir) + i = 0 + with io.open(fd, mode="wb") as fp: + with io.open(fname, mode="rb") as fi: + partial_line = False + while True: + line = fi.readline(block_size) + if not line: + if partial_line: + fp.write(b"\n") + i += 1 + break - # We have a block... maybe at the end of the file. - partial_line = True - fp.write(line) + strip_line = line.rstrip(b"\r\n") + if len(strip_line) != len(line): + partial_line = False + fp.write(strip_line) + fp.write(b"\n") + i += 1 + continue + + # We have a block... maybe at the end of the file. + partial_line = True + fp.write(line) if in_place: shutil.move(temp_name, fname) diff --git a/lib/galaxy/datatypes/test/1.imzml b/lib/galaxy/datatypes/test/1.imzml new file mode 100644 index 00000000000..6b45d2a3667 --- /dev/null +++ b/lib/galaxy/datatypes/test/1.imzml @@ -0,0 +1,380 @@ + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + diff --git a/lib/galaxy/datatypes/test/dosimzml b/lib/galaxy/datatypes/test/dosimzml new file mode 100644 index 00000000000..6ff4f5c078d --- /dev/null +++ b/lib/galaxy/datatypes/test/dosimzml @@ -0,0 +1,380 @@ + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + diff --git a/test/unit/datatypes/test_sniff.py b/test/unit/datatypes/test_sniff.py index a41f891dba3..7f6370f7df9 100644 --- a/test/unit/datatypes/test_sniff.py +++ b/test/unit/datatypes/test_sniff.py @@ -1,6 +1,11 @@ import tempfile -from galaxy.datatypes.sniff import convert_newlines_sep2tabs, sep2tabs +from galaxy.datatypes.sniff import ( + convert_newlines, + convert_newlines_sep2tabs, + get_test_fname, + sep2tabs, +) def assert_converts_to_1234_sep2tabs(content, line_ending="\n"): @@ -23,6 +28,13 @@ def assert_converts_to_1234_convert_sep2tabs(content, expected='1\t2\n3\t4\n', l assert expected == open(tf.name).read() +def test_convert_newlines_non_utf(): + fname = get_test_fname("dosimzml") + rval = convert_newlines(fname, tmp_prefix="gxtest", tmp_dir=tempfile.gettempdir(), in_place=False) + new_file = rval[1] + assert open(new_file, "rb").read() == open(get_test_fname("1.imzml"), "rb").read() + + def test_sep2tabs(): assert_converts_to_1234_sep2tabs("1 2\n3 4\n") assert_converts_to_1234_sep2tabs("1 2\n3 4\n") From ed246645d996f9cd57345de09089b0b5d2c0f026 Mon Sep 17 00:00:00 2001 From: John Chilton Date: Thu, 16 May 2019 09:57:03 -0400 Subject: [PATCH 04/14] Binary upload fix for interaction with FilePrefix stuff. --- lib/galaxy/datatypes/sniff.py | 25 ++++++++++++------- lib/galaxy/datatypes/test/{1.imzml => 1imzml} | 0 2 files changed, 16 insertions(+), 9 deletions(-) rename lib/galaxy/datatypes/test/{1.imzml => 1imzml} (100%) diff --git a/lib/galaxy/datatypes/sniff.py b/lib/galaxy/datatypes/sniff.py index a629fc15695..8c08d1ebf6a 100644 --- a/lib/galaxy/datatypes/sniff.py +++ b/lib/galaxy/datatypes/sniff.py @@ -536,6 +536,9 @@ def guess_ext(fname, sniff_order, is_binary=False): >>> fname = get_test_fname('1.mtx') >>> guess_ext(fname, sniff_order) 'mtx' + >>> fname = get_test_fname('1imzml') + >>> guess_ext(fname, sniff_order) # This test case is ensuring doesn't throw exception, actual value could change if non-utf encoding handling improves. + 'data' """ file_prefix = FilePrefix(fname) file_ext = run_sniffers_raw(file_prefix, sniff_order, is_binary) @@ -617,8 +620,9 @@ def zip_single_fileobj(path): class FilePrefix(object): def __init__(self, filename): - binary = False + non_utf8_error = None compressed_format = None + contents_header_bytes = None contents_header = None # First MAX_BYTES of the file. truncated = False # A future direction to optimize sniffing even more for sniffers at the top of the list @@ -627,20 +631,23 @@ class FilePrefix(object): # populates contents_header while providing a StringIO-like interface until the file is read # but then would fallback to native string_io() try: - compressed_format, f = compression_utils.get_fileobj_raw(filename) + compressed_format, f = compression_utils.get_fileobj_raw(filename, "rb") try: - contents_header = f.read(SNIFF_PREFIX_BYTES) - truncated = len(contents_header) == SNIFF_PREFIX_BYTES + contents_header_bytes = f.read(SNIFF_PREFIX_BYTES) + truncated = len(contents_header_bytes) == SNIFF_PREFIX_BYTES + contents_header = contents_header_bytes.decode("utf-8") finally: f.close() - except UnicodeDecodeError: - binary = True + except UnicodeDecodeError as e: + non_utf8_error = e self.truncated = truncated self.filename = filename - self.binary = binary + self.non_utf8_error = non_utf8_error + self.binary = non_utf8_error is not None # obviously wrong self.compressed_format = compressed_format self.contents_header = contents_header + self.contents_header_bytes = contents_header_bytes self._file_size = None @property @@ -650,8 +657,8 @@ class FilePrefix(object): return self._file_size def string_io(self): - if self.binary: - raise Exception("Attempting to create a StringIO object for binary data.") + if self.non_utf8_error is not None: + raise self.non_utf8_error rval = StringIO(self.contents_header) return rval diff --git a/lib/galaxy/datatypes/test/1.imzml b/lib/galaxy/datatypes/test/1imzml similarity index 100% rename from lib/galaxy/datatypes/test/1.imzml rename to lib/galaxy/datatypes/test/1imzml From 28c0a3b077bf0c56b8c22e39b945da43e5471b48 Mon Sep 17 00:00:00 2001 From: John Chilton Date: Thu, 16 May 2019 15:10:22 -0400 Subject: [PATCH 05/14] Improve sniffing test cases. --- lib/galaxy/datatypes/sniff.py | 29 ------------------------- test/unit/datatypes/test_sniff.py | 35 ++++++++++++++++++++++++++++++- 2 files changed, 34 insertions(+), 30 deletions(-) diff --git a/lib/galaxy/datatypes/sniff.py b/lib/galaxy/datatypes/sniff.py index 8c08d1ebf6a..7fb42ddf339 100644 --- a/lib/galaxy/datatypes/sniff.py +++ b/lib/galaxy/datatypes/sniff.py @@ -107,35 +107,6 @@ def convert_newlines(fname, in_place=True, tmp_dir=None, tmp_prefix="gxupload", """ Converts in place a file from universal line endings to Posix line endings. - - >>> def assert_converts_to_1234(content, block_size=1024): - ... fname = get_test_fname('temp.txt') - ... with open(fname, 'w') as fh: - ... _ = fh.write(content) - ... rval = convert_newlines(fname, tmp_prefix="gxtest", tmp_dir=tempfile.gettempdir(), block_size=block_size) - ... assert rval == (2, None), rval - ... actual_contents = open(fname).read() - ... assert '1 2\\n3 4\\n' == actual_contents, actual_contents - >>> # Verify ends with newline - with or without that on inputs - for any of - >>> # \\r \\n or \\r\\n newlines. - >>> assert_converts_to_1234("1 2\\r3 4") - >>> assert_converts_to_1234("1 2\\n3 4") - >>> assert_converts_to_1234("1 2\\r\\n3 4") - >>> assert_converts_to_1234("1 2\\r3 4\\r") - >>> assert_converts_to_1234("1 2\\n3 4\\n") - >>> assert_converts_to_1234("1 2\\r\\n3 4\\r\\n") - >>> assert_converts_to_1234("1 2\\r3 4", block_size=2) - >>> assert_converts_to_1234("1 2\\n3 4", block_size=2) - >>> assert_converts_to_1234("1 2\\r\\n3 4", block_size=2) - >>> assert_converts_to_1234("1 2\\r3 4\\r", block_size=2) - >>> assert_converts_to_1234("1 2\\n3 4\\n", block_size=2) - >>> assert_converts_to_1234("1 2\\r\\n3 4\\r\\n", block_size=2) - >>> assert_converts_to_1234("1 2\\r3 4", block_size=3) - >>> assert_converts_to_1234("1 2\\n3 4", block_size=3) - >>> assert_converts_to_1234("1 2\\r\\n3 4", block_size=3) - >>> assert_converts_to_1234("1 2\\r3 4\\r", block_size=3) - >>> assert_converts_to_1234("1 2\\n3 4\\n", block_size=3) - >>> assert_converts_to_1234("1 2\\r\\n3 4\\r\\n", block_size=3) """ fd, temp_name = tempfile.mkstemp(prefix=tmp_prefix, dir=tmp_dir) i = 0 diff --git a/test/unit/datatypes/test_sniff.py b/test/unit/datatypes/test_sniff.py index 7f6370f7df9..2d10dc21ff1 100644 --- a/test/unit/datatypes/test_sniff.py +++ b/test/unit/datatypes/test_sniff.py @@ -28,11 +28,44 @@ def assert_converts_to_1234_convert_sep2tabs(content, expected='1\t2\n3\t4\n', l assert expected == open(tf.name).read() +def assert_converts_to_1234_convert(content, block_size=1024): + fname = get_test_fname('temp2.txt') + with open(fname, 'w') as fh: + _ = fh.write(content) + rval = convert_newlines(fname, tmp_prefix="gxtest", tmp_dir=tempfile.gettempdir(), block_size=block_size) + #assert rval == (2, None), "rval != %s for %s" % (rval, content) + actual_contents = open(fname).read() + assert '1 2\n3 4\n' == actual_contents, actual_contents + + +def test_convert_newlines(): + # Verify ends with newline - with or without that on inputs - for any of + # \r \\n or \\r\\n newlines. + assert_converts_to_1234_convert("1 2\r3 4") + assert_converts_to_1234_convert("1 2\n3 4") + assert_converts_to_1234_convert("1 2\r\n3 4") + assert_converts_to_1234_convert("1 2\r3 4\r") + assert_converts_to_1234_convert("1 2\n3 4\n") + assert_converts_to_1234_convert("1 2\r\n3 4\r\n") + assert_converts_to_1234_convert("1 2\r3 4", block_size=2) + assert_converts_to_1234_convert("1 2\n3 4", block_size=2) + assert_converts_to_1234_convert("1 2\r\n3 4", block_size=2) + assert_converts_to_1234_convert("1 2\r3 4\r", block_size=2) + assert_converts_to_1234_convert("1 2\n3 4\n", block_size=2) + assert_converts_to_1234_convert("1 2\r\n3 4\r\n", block_size=2) + assert_converts_to_1234_convert("1 2\r3 4", block_size=3) + assert_converts_to_1234_convert("1 2\n3 4", block_size=3) + assert_converts_to_1234_convert("1 2\r\n3 4", block_size=3) + assert_converts_to_1234_convert("1 2\r3 4\r", block_size=3) + assert_converts_to_1234_convert("1 2\n3 4\n", block_size=3) + assert_converts_to_1234_convert("1 2\r\n3 4\r\n", block_size=3) + + def test_convert_newlines_non_utf(): fname = get_test_fname("dosimzml") rval = convert_newlines(fname, tmp_prefix="gxtest", tmp_dir=tempfile.gettempdir(), in_place=False) new_file = rval[1] - assert open(new_file, "rb").read() == open(get_test_fname("1.imzml"), "rb").read() + assert open(new_file, "rb").read() == open(get_test_fname("1imzml"), "rb").read() def test_sep2tabs(): From 78f0a8893c59cf144ac1c5e9421c2ee57226f5af Mon Sep 17 00:00:00 2001 From: John Chilton Date: Thu, 16 May 2019 15:12:14 -0400 Subject: [PATCH 06/14] Binary variant of convert_newlines. --- lib/galaxy/datatypes/sniff.py | 51 +++-------------------------------- 1 file changed, 4 insertions(+), 47 deletions(-) diff --git a/lib/galaxy/datatypes/sniff.py b/lib/galaxy/datatypes/sniff.py index 7fb42ddf339..f3c2e25cf8d 100644 --- a/lib/galaxy/datatypes/sniff.py +++ b/lib/galaxy/datatypes/sniff.py @@ -110,53 +110,10 @@ def convert_newlines(fname, in_place=True, tmp_dir=None, tmp_prefix="gxupload", """ fd, temp_name = tempfile.mkstemp(prefix=tmp_prefix, dir=tmp_dir) i = 0 - try: - with io.open(fd, mode="wt", encoding='utf-8') as fp: - with io.open(fname, encoding='utf-8') as fi: - partial_line = False - while True: - line = fi.readline(block_size) - if not line: - if partial_line: - fp.write(u"\n") - i += 1 - break - - if line[-1] == u"\n": - partial_line = False - fp.write(line) - i += 1 - continue - - # We have a block... maybe at the end of the file. - partial_line = True - fp.write(line) - except UnicodeDecodeError: - fd, temp_name = tempfile.mkstemp(prefix=tmp_prefix, dir=tmp_dir) - i = 0 - with io.open(fd, mode="wb") as fp: - with io.open(fname, mode="rb") as fi: - partial_line = False - while True: - line = fi.readline(block_size) - if not line: - if partial_line: - fp.write(b"\n") - i += 1 - break - - strip_line = line.rstrip(b"\r\n") - if len(strip_line) != len(line): - partial_line = False - fp.write(strip_line) - fp.write(b"\n") - i += 1 - continue - - # We have a block... maybe at the end of the file. - partial_line = True - fp.write(line) - + with io.open(fd, mode="wb") as fp: + with io.open(fname, mode="rb") as fi: + for i, line in enumerate(fi): + fp.write(b"%s\n" % line.rstrip(b"\r\n")) if in_place: shutil.move(temp_name, fname) # Return number of lines in file. From ae1922136383c10ee5813ab6aabb25cdd2b6f952 Mon Sep 17 00:00:00 2001 From: mvdbeek Date: Fri, 17 May 2019 10:42:20 +0200 Subject: [PATCH 07/14] Use byte replacement strategy --- lib/galaxy/datatypes/sniff.py | 20 ++++++++++++++++++-- 1 file changed, 18 insertions(+), 2 deletions(-) diff --git a/lib/galaxy/datatypes/sniff.py b/lib/galaxy/datatypes/sniff.py index f3c2e25cf8d..e6880fa7e35 100644 --- a/lib/galaxy/datatypes/sniff.py +++ b/lib/galaxy/datatypes/sniff.py @@ -112,8 +112,24 @@ def convert_newlines(fname, in_place=True, tmp_dir=None, tmp_prefix="gxupload", i = 0 with io.open(fd, mode="wb") as fp: with io.open(fname, mode="rb") as fi: - for i, line in enumerate(fi): - fp.write(b"%s\n" % line.rstrip(b"\r\n")) + line = b'' + clean_line = b"" + last_char = None + block = fi.read(block_size) + while block: + if last_char == "\r" and block.startswith(b"\n"): + # last block ended with "\r", new block startswith "\n" + # since we replace "\r" with "\n" in the previous iteration we skip the first byte + block = block[1:] + # splitlines(True) splits at line terminators but keeps them so we can replace them + lines = block.splitlines(True) + for line in lines: + clean_line = line.replace(b"\r\n", b"\n").replace(b"\r", b"\n") + fp.write(clean_line) + last_char = util.unicodify(line, error='replace')[-1] + block = fi.read(block_size) + if not clean_line.endswith(b"\n"): + fp.write(b"\n") if in_place: shutil.move(temp_name, fname) # Return number of lines in file. From 1d967dc7d0041b09fe5a0c33a32c5ab33ebe039f Mon Sep 17 00:00:00 2001 From: mvdbeek Date: Fri, 17 May 2019 10:42:43 +0200 Subject: [PATCH 08/14] Fix test_sniff unittest --- test/unit/datatypes/test_sniff.py | 19 ++++++++----------- 1 file changed, 8 insertions(+), 11 deletions(-) diff --git a/test/unit/datatypes/test_sniff.py b/test/unit/datatypes/test_sniff.py index 2d10dc21ff1..ff98f2732cb 100644 --- a/test/unit/datatypes/test_sniff.py +++ b/test/unit/datatypes/test_sniff.py @@ -1,3 +1,4 @@ +import io import tempfile from galaxy.datatypes.sniff import ( @@ -9,20 +10,16 @@ from galaxy.datatypes.sniff import ( def assert_converts_to_1234_sep2tabs(content, line_ending="\n"): - print("\r" in content) - tf = tempfile.NamedTemporaryFile(delete=False, mode='w') - tf.write(content) - tf.close() + with tempfile.NamedTemporaryFile(delete=False, mode='w') as tf: + tf.write(content) rval = sep2tabs(tf.name, tmp_prefix="gxtest", tmp_dir=tempfile.gettempdir()) assert rval == (2, None), rval - assert '1\t2%s3\t4%s' % (line_ending, line_ending) == open(tf.name).read() + assert '1\t2%s3\t4%s' % (line_ending, line_ending) == io.open(tf.name, newline='').read() def assert_converts_to_1234_convert_sep2tabs(content, expected='1\t2\n3\t4\n', line_ending="\n"): - print("\r" in content) - tf = tempfile.NamedTemporaryFile(delete=False, mode='w') - tf.write(content) - tf.close() + with tempfile.NamedTemporaryFile(delete=False, mode='w') as tf: + tf.write(content) rval = convert_newlines_sep2tabs(tf.name, tmp_prefix="gxtest", tmp_dir=tempfile.gettempdir()) assert rval == (2, None), rval assert expected == open(tf.name).read() @@ -31,9 +28,9 @@ def assert_converts_to_1234_convert_sep2tabs(content, expected='1\t2\n3\t4\n', l def assert_converts_to_1234_convert(content, block_size=1024): fname = get_test_fname('temp2.txt') with open(fname, 'w') as fh: - _ = fh.write(content) + fh.write(content) rval = convert_newlines(fname, tmp_prefix="gxtest", tmp_dir=tempfile.gettempdir(), block_size=block_size) - #assert rval == (2, None), "rval != %s for %s" % (rval, content) + assert rval == (2, None), "rval != %s for %s" % (rval, content) actual_contents = open(fname).read() assert '1 2\n3 4\n' == actual_contents, actual_contents From 06093c24b7f5c165cd904c97fc30bd2a37a21aa5 Mon Sep 17 00:00:00 2001 From: mvdbeek Date: Fri, 17 May 2019 10:58:36 +0200 Subject: [PATCH 09/14] Fix line counting --- lib/galaxy/datatypes/sniff.py | 11 +++++++---- 1 file changed, 7 insertions(+), 4 deletions(-) diff --git a/lib/galaxy/datatypes/sniff.py b/lib/galaxy/datatypes/sniff.py index e6880fa7e35..6d352abb08d 100644 --- a/lib/galaxy/datatypes/sniff.py +++ b/lib/galaxy/datatypes/sniff.py @@ -113,7 +113,7 @@ def convert_newlines(fname, in_place=True, tmp_dir=None, tmp_prefix="gxupload", with io.open(fd, mode="wb") as fp: with io.open(fname, mode="rb") as fi: line = b'' - clean_line = b"" + converted_line = b"" last_char = None block = fi.read(block_size) while block: @@ -124,11 +124,14 @@ def convert_newlines(fname, in_place=True, tmp_dir=None, tmp_prefix="gxupload", # splitlines(True) splits at line terminators but keeps them so we can replace them lines = block.splitlines(True) for line in lines: - clean_line = line.replace(b"\r\n", b"\n").replace(b"\r", b"\n") - fp.write(clean_line) + converted_line = line.replace(b"\r\n", b"\n").replace(b"\r", b"\n") + if b"\n" in converted_line: + i += 1 + fp.write(converted_line) last_char = util.unicodify(line, error='replace')[-1] block = fi.read(block_size) - if not clean_line.endswith(b"\n"): + if not converted_line.endswith(b"\n"): + i += 1 fp.write(b"\n") if in_place: shutil.move(temp_name, fname) From da4887877da6e4c9011ca084742d2c5bcc025760 Mon Sep 17 00:00:00 2001 From: mvdbeek Date: Fri, 17 May 2019 12:18:50 +0200 Subject: [PATCH 10/14] Parametrize sniff_test --- test/unit/datatypes/test_sniff.py | 86 +++++++++++++++++++------------ 1 file changed, 53 insertions(+), 33 deletions(-) diff --git a/test/unit/datatypes/test_sniff.py b/test/unit/datatypes/test_sniff.py index ff98f2732cb..4df85cdb9fa 100644 --- a/test/unit/datatypes/test_sniff.py +++ b/test/unit/datatypes/test_sniff.py @@ -1,6 +1,8 @@ import io import tempfile +import pytest + from galaxy.datatypes.sniff import ( convert_newlines, convert_newlines_sep2tabs, @@ -35,27 +37,33 @@ def assert_converts_to_1234_convert(content, block_size=1024): assert '1 2\n3 4\n' == actual_contents, actual_contents -def test_convert_newlines(): +@pytest.mark.parametrize('source,block_size', [ + ("1 2\r3 4", None), + ("1 2\n3 4", None), + ("1 2\r\n3 4", None), + ("1 2\r3 4\r", None), + ("1 2\n3 4\n", None), + ("1 2\r\n3 4\r\n", None), + ("1 2\r3 4", 2), + ("1 2\n3 4", 2), + ("1 2\r\n3 4", 2), + ("1 2\r3 4\r", 2), + ("1 2\n3 4\n", 2), + ("1 2\r\n3 4\r\n", 2), + ("1 2\r3 4", 3), + ("1 2\n3 4", 3), + ("1 2\r\n3 4", 3), + ("1 2\r3 4\r", 3), + ("1 2\n3 4\n", 3), + ("1 2\r\n3 4\r\n", 3), +]) +def test_convert_newlines(source, block_size): # Verify ends with newline - with or without that on inputs - for any of # \r \\n or \\r\\n newlines. - assert_converts_to_1234_convert("1 2\r3 4") - assert_converts_to_1234_convert("1 2\n3 4") - assert_converts_to_1234_convert("1 2\r\n3 4") - assert_converts_to_1234_convert("1 2\r3 4\r") - assert_converts_to_1234_convert("1 2\n3 4\n") - assert_converts_to_1234_convert("1 2\r\n3 4\r\n") - assert_converts_to_1234_convert("1 2\r3 4", block_size=2) - assert_converts_to_1234_convert("1 2\n3 4", block_size=2) - assert_converts_to_1234_convert("1 2\r\n3 4", block_size=2) - assert_converts_to_1234_convert("1 2\r3 4\r", block_size=2) - assert_converts_to_1234_convert("1 2\n3 4\n", block_size=2) - assert_converts_to_1234_convert("1 2\r\n3 4\r\n", block_size=2) - assert_converts_to_1234_convert("1 2\r3 4", block_size=3) - assert_converts_to_1234_convert("1 2\n3 4", block_size=3) - assert_converts_to_1234_convert("1 2\r\n3 4", block_size=3) - assert_converts_to_1234_convert("1 2\r3 4\r", block_size=3) - assert_converts_to_1234_convert("1 2\n3 4\n", block_size=3) - assert_converts_to_1234_convert("1 2\r\n3 4\r\n", block_size=3) + if block_size: + assert_converts_to_1234_convert(source, block_size) + else: + assert_converts_to_1234_convert(source) def test_convert_newlines_non_utf(): @@ -65,19 +73,31 @@ def test_convert_newlines_non_utf(): assert open(new_file, "rb").read() == open(get_test_fname("1imzml"), "rb").read() -def test_sep2tabs(): - assert_converts_to_1234_sep2tabs("1 2\n3 4\n") - assert_converts_to_1234_sep2tabs("1 2\n3 4\n") - assert_converts_to_1234_sep2tabs("1\t2\n3\t4\n") - assert_converts_to_1234_sep2tabs("1\t2\r3\t4\r", line_ending='\r') - assert_converts_to_1234_sep2tabs("1\t2\r\n3\t4\r\n", line_ending='\r\n') +@pytest.mark.parametrize('source,line_ending', [ + ("1 2\n3 4\n", None), + ("1 2\n3 4\n", None), + ("1\t2\n3\t4\n", None), + ("1\t2\r3\t4\r", '\r'), + ("1\t2\r\n3\t4\r\n", '\r\n'), +]) +def test_sep2tabs(source, line_ending): + if line_ending: + assert_converts_to_1234_sep2tabs(source, line_ending) + else: + assert_converts_to_1234_sep2tabs(source) -def test_convert_sep2tabs(): - assert_converts_to_1234_convert_sep2tabs("1 2\n3 4\n") - assert_converts_to_1234_convert_sep2tabs("1 2\n3 4\n") - assert_converts_to_1234_convert_sep2tabs("1\t2\n3\t4\n") - assert_converts_to_1234_convert_sep2tabs("1\t2\r3\t4\r") - assert_converts_to_1234_convert_sep2tabs("1\t2\r\n3\t4\r\n") - assert_converts_to_1234_convert_sep2tabs("1 2\r\n3 4\r\n") - assert_converts_to_1234_convert_sep2tabs("1 2 \n3 4 \n", expected='1\t2\t\n3\t4\t\n') +@pytest.mark.parametrize('source,expected', [ + ("1 2\n3 4\n", None), + ("1 2\n3 4\n", None), + ("1\t2\n3\t4\n", None), + ("1\t2\r3\t4\r", None), + ("1\t2\r\n3\t4\r\n", None), + ("1 2\r\n3 4\r\n", None), + ("1 2 \n3 4 \n", '1\t2\t\n3\t4\t\n'), +]) +def test_convert_sep2tabs(source, expected): + if expected: + assert_converts_to_1234_convert_sep2tabs(source, expected=expected) + else: + assert_converts_to_1234_convert_sep2tabs(source) From 859734e50c0b8a7763cddb78acbce3f4c89dd07c Mon Sep 17 00:00:00 2001 From: mvdbeek Date: Fri, 17 May 2019 11:24:44 +0200 Subject: [PATCH 11/14] Operate on blocks directly --- lib/galaxy/datatypes/sniff.py | 39 +++++++++++++++++++++-------------- 1 file changed, 23 insertions(+), 16 deletions(-) diff --git a/lib/galaxy/datatypes/sniff.py b/lib/galaxy/datatypes/sniff.py index 6d352abb08d..35f7dba4a5f 100644 --- a/lib/galaxy/datatypes/sniff.py +++ b/lib/galaxy/datatypes/sniff.py @@ -14,7 +14,11 @@ import sys import tempfile import zipfile -from six import StringIO, text_type +from six import ( + PY3, + StringIO, + text_type, +) from six.moves import filter from six.moves.urllib.request import urlopen @@ -110,27 +114,30 @@ def convert_newlines(fname, in_place=True, tmp_dir=None, tmp_prefix="gxupload", """ fd, temp_name = tempfile.mkstemp(prefix=tmp_prefix, dir=tmp_dir) i = 0 + if PY3: + NEWLINE_BYTE = 10 + CR_BYTE = 13 + else: + NEWLINE_BYTE = "\n" + CR_BYTE = "\r" with io.open(fd, mode="wb") as fp: with io.open(fname, mode="rb") as fi: - line = b'' - converted_line = b"" last_char = None block = fi.read(block_size) + last_block = b"" while block: - if last_char == "\r" and block.startswith(b"\n"): - # last block ended with "\r", new block startswith "\n" - # since we replace "\r" with "\n" in the previous iteration we skip the first byte + if last_char == CR_BYTE and block.startswith(b"\n"): + # Last block ended with CR, new block startswith newline. + # Since we replace CR with newline in the previous iteration we skip the first byte block = block[1:] - # splitlines(True) splits at line terminators but keeps them so we can replace them - lines = block.splitlines(True) - for line in lines: - converted_line = line.replace(b"\r\n", b"\n").replace(b"\r", b"\n") - if b"\n" in converted_line: - i += 1 - fp.write(converted_line) - last_char = util.unicodify(line, error='replace')[-1] - block = fi.read(block_size) - if not converted_line.endswith(b"\n"): + if block: + last_char = block[-1] + block = block.replace(b"\r\n", b"\n").replace(b"\r", b"\n") + fp.write(block) + i += block.count(b"\n") + last_block = block + block = fi.read(block_size) + if last_block and last_block[-1] != NEWLINE_BYTE: i += 1 fp.write(b"\n") if in_place: From 2e6743e89753fd18812bc1e7adb6666c3cb9f3de Mon Sep 17 00:00:00 2001 From: mvdbeek Date: Fri, 17 May 2019 16:14:44 +0200 Subject: [PATCH 12/14] Drop (seemingly?) unused sep2tabs function --- lib/galaxy/datatypes/sniff.py | 33 ------------------------------- test/unit/datatypes/test_sniff.py | 23 --------------------- 2 files changed, 56 deletions(-) diff --git a/lib/galaxy/datatypes/sniff.py b/lib/galaxy/datatypes/sniff.py index 35f7dba4a5f..2048291a9c9 100644 --- a/lib/galaxy/datatypes/sniff.py +++ b/lib/galaxy/datatypes/sniff.py @@ -148,39 +148,6 @@ def convert_newlines(fname, in_place=True, tmp_dir=None, tmp_prefix="gxupload", return (i, temp_name) -def sep2tabs(fname, in_place=True, patt=r"\s+", tmp_dir=None, tmp_prefix="gxupload"): - """ - Transforms in place a 'sep' separated file to a tab separated one - """ - regexp = re.compile(patt) - fd, temp_name = tempfile.mkstemp(prefix=tmp_prefix, dir=tmp_dir) - with io.open(fd, mode="w", encoding='utf-8') as fp: - i = None - for i, line in enumerate(io.open(fname, encoding='utf-8', newline='')): - if line.endswith("\r"): - line = line.rstrip('\r') - elems = regexp.split(line) - fp.write(u"%s\r" % '\t'.join(elems)) - elif line.endswith("\r\n"): - line = line.rstrip('\r\n') - elems = regexp.split(line) - fp.write(u"%s\r\n" % '\t'.join(elems)) - else: - line = line.rstrip('\n') - elems = regexp.split(line) - fp.write(u"%s\n" % '\t'.join(elems)) - if i is None: - i = 0 - else: - i += 1 - if in_place: - shutil.move(temp_name, fname) - # Return number of lines in file. - return (i, None) - else: - return (i, temp_name) - - def convert_newlines_sep2tabs(fname, in_place=True, patt=r"\s+", tmp_dir=None, tmp_prefix="gxupload"): """ Combines above methods: convert_newlines() and sep2tabs() diff --git a/test/unit/datatypes/test_sniff.py b/test/unit/datatypes/test_sniff.py index 4df85cdb9fa..fab1794c8d0 100644 --- a/test/unit/datatypes/test_sniff.py +++ b/test/unit/datatypes/test_sniff.py @@ -7,18 +7,9 @@ from galaxy.datatypes.sniff import ( convert_newlines, convert_newlines_sep2tabs, get_test_fname, - sep2tabs, ) -def assert_converts_to_1234_sep2tabs(content, line_ending="\n"): - with tempfile.NamedTemporaryFile(delete=False, mode='w') as tf: - tf.write(content) - rval = sep2tabs(tf.name, tmp_prefix="gxtest", tmp_dir=tempfile.gettempdir()) - assert rval == (2, None), rval - assert '1\t2%s3\t4%s' % (line_ending, line_ending) == io.open(tf.name, newline='').read() - - def assert_converts_to_1234_convert_sep2tabs(content, expected='1\t2\n3\t4\n', line_ending="\n"): with tempfile.NamedTemporaryFile(delete=False, mode='w') as tf: tf.write(content) @@ -73,20 +64,6 @@ def test_convert_newlines_non_utf(): assert open(new_file, "rb").read() == open(get_test_fname("1imzml"), "rb").read() -@pytest.mark.parametrize('source,line_ending', [ - ("1 2\n3 4\n", None), - ("1 2\n3 4\n", None), - ("1\t2\n3\t4\n", None), - ("1\t2\r3\t4\r", '\r'), - ("1\t2\r\n3\t4\r\n", '\r\n'), -]) -def test_sep2tabs(source, line_ending): - if line_ending: - assert_converts_to_1234_sep2tabs(source, line_ending) - else: - assert_converts_to_1234_sep2tabs(source) - - @pytest.mark.parametrize('source,expected', [ ("1 2\n3 4\n", None), ("1 2\n3 4\n", None), From 5ab6e799f85ae77cb01ab3f58404c8fa153a1b08 Mon Sep 17 00:00:00 2001 From: mvdbeek Date: Fri, 17 May 2019 17:38:40 +0200 Subject: [PATCH 13/14] Simplify convert_newlines_sep2tabs Just call convert_newlines with a compiled regex --- lib/galaxy/datatypes/sniff.py | 24 +++++------------------- test/unit/datatypes/test_sniff.py | 7 +++---- 2 files changed, 8 insertions(+), 23 deletions(-) diff --git a/lib/galaxy/datatypes/sniff.py b/lib/galaxy/datatypes/sniff.py index 2048291a9c9..ff2d69acc7b 100644 --- a/lib/galaxy/datatypes/sniff.py +++ b/lib/galaxy/datatypes/sniff.py @@ -107,7 +107,7 @@ def stream_to_file(stream, suffix='', prefix='', dir=None, text=False, **kwd): return stream_to_open_named_file(stream, fd, temp_name, **kwd) -def convert_newlines(fname, in_place=True, tmp_dir=None, tmp_prefix="gxupload", block_size=128 * 1024): +def convert_newlines(fname, in_place=True, tmp_dir=None, tmp_prefix="gxupload", block_size=128 * 1024, regexp=None): """ Converts in place a file from universal line endings to Posix line endings. @@ -133,6 +133,8 @@ def convert_newlines(fname, in_place=True, tmp_dir=None, tmp_prefix="gxupload", if block: last_char = block[-1] block = block.replace(b"\r\n", b"\n").replace(b"\r", b"\n") + if regexp: + block = b"\t".join(regexp.split(block)) fp.write(block) i += block.count(b"\n") last_block = block @@ -148,7 +150,7 @@ def convert_newlines(fname, in_place=True, tmp_dir=None, tmp_prefix="gxupload", return (i, temp_name) -def convert_newlines_sep2tabs(fname, in_place=True, patt=r"\s+", tmp_dir=None, tmp_prefix="gxupload"): +def convert_newlines_sep2tabs(fname, in_place=True, patt=br"[^\S\n]+", tmp_dir=None, tmp_prefix="gxupload"): """ Combines above methods: convert_newlines() and sep2tabs() so that files do not need to be read twice @@ -162,23 +164,7 @@ def convert_newlines_sep2tabs(fname, in_place=True, patt=r"\s+", tmp_dir=None, t '1\\t2\\n3\\t4\\n' """ regexp = re.compile(patt) - fd, temp_name = tempfile.mkstemp(prefix=tmp_prefix, dir=tmp_dir) - with io.open(fd, mode="wt", encoding='utf-8') as fp: - i = None - for i, line in enumerate(io.open(fname, encoding='utf-8')): - line = line.rstrip('\r\n') - elems = regexp.split(line) - fp.write(u"%s\n" % '\t'.join(elems)) - if i is None: - i = 0 - else: - i = i + 1 - if in_place: - shutil.move(temp_name, fname) - # Return number of lines in file. - return (i, None) - else: - return (i, temp_name) + return convert_newlines(fname, in_place, tmp_dir, tmp_prefix, regexp=regexp) def iter_headers(fname_or_file_prefix, sep, count=60, comment_designator=None): diff --git a/test/unit/datatypes/test_sniff.py b/test/unit/datatypes/test_sniff.py index fab1794c8d0..faf75443281 100644 --- a/test/unit/datatypes/test_sniff.py +++ b/test/unit/datatypes/test_sniff.py @@ -1,4 +1,3 @@ -import io import tempfile import pytest @@ -10,12 +9,12 @@ from galaxy.datatypes.sniff import ( ) -def assert_converts_to_1234_convert_sep2tabs(content, expected='1\t2\n3\t4\n', line_ending="\n"): +def assert_converts_to_1234_convert_sep2tabs(content, expected='1\t2\n3\t4\n'): with tempfile.NamedTemporaryFile(delete=False, mode='w') as tf: tf.write(content) rval = convert_newlines_sep2tabs(tf.name, tmp_prefix="gxtest", tmp_dir=tempfile.gettempdir()) - assert rval == (2, None), rval assert expected == open(tf.name).read() + assert rval == (2, None), rval def assert_converts_to_1234_convert(content, block_size=1024): @@ -23,9 +22,9 @@ def assert_converts_to_1234_convert(content, block_size=1024): with open(fname, 'w') as fh: fh.write(content) rval = convert_newlines(fname, tmp_prefix="gxtest", tmp_dir=tempfile.gettempdir(), block_size=block_size) - assert rval == (2, None), "rval != %s for %s" % (rval, content) actual_contents = open(fname).read() assert '1 2\n3 4\n' == actual_contents, actual_contents + assert rval == (2, None), "rval != %s for %s" % (rval, content) @pytest.mark.parametrize('source,block_size', [ From 733cb5eb42b1dee9a620769e4a1999f83d8674c1 Mon Sep 17 00:00:00 2001 From: mvdbeek Date: Fri, 17 May 2019 17:53:47 +0200 Subject: [PATCH 14/14] Group with statements and clean up docstring --- lib/galaxy/datatypes/sniff.py | 46 +++++++++++++++++------------------ 1 file changed, 22 insertions(+), 24 deletions(-) diff --git a/lib/galaxy/datatypes/sniff.py b/lib/galaxy/datatypes/sniff.py index ff2d69acc7b..d35c83ffd4c 100644 --- a/lib/galaxy/datatypes/sniff.py +++ b/lib/galaxy/datatypes/sniff.py @@ -120,28 +120,27 @@ def convert_newlines(fname, in_place=True, tmp_dir=None, tmp_prefix="gxupload", else: NEWLINE_BYTE = "\n" CR_BYTE = "\r" - with io.open(fd, mode="wb") as fp: - with io.open(fname, mode="rb") as fi: - last_char = None - block = fi.read(block_size) - last_block = b"" - while block: - if last_char == CR_BYTE and block.startswith(b"\n"): - # Last block ended with CR, new block startswith newline. - # Since we replace CR with newline in the previous iteration we skip the first byte - block = block[1:] - if block: - last_char = block[-1] - block = block.replace(b"\r\n", b"\n").replace(b"\r", b"\n") - if regexp: - block = b"\t".join(regexp.split(block)) - fp.write(block) - i += block.count(b"\n") - last_block = block - block = fi.read(block_size) - if last_block and last_block[-1] != NEWLINE_BYTE: - i += 1 - fp.write(b"\n") + with io.open(fd, mode="wb") as fp, io.open(fname, mode="rb") as fi: + last_char = None + block = fi.read(block_size) + last_block = b"" + while block: + if last_char == CR_BYTE and block.startswith(b"\n"): + # Last block ended with CR, new block startswith newline. + # Since we replace CR with newline in the previous iteration we skip the first byte + block = block[1:] + if block: + last_char = block[-1] + block = block.replace(b"\r\n", b"\n").replace(b"\r", b"\n") + if regexp: + block = b"\t".join(regexp.split(block)) + fp.write(block) + i += block.count(b"\n") + last_block = block + block = fi.read(block_size) + if last_block and last_block[-1] != NEWLINE_BYTE: + i += 1 + fp.write(b"\n") if in_place: shutil.move(temp_name, fname) # Return number of lines in file. @@ -152,8 +151,7 @@ def convert_newlines(fname, in_place=True, tmp_dir=None, tmp_prefix="gxupload", def convert_newlines_sep2tabs(fname, in_place=True, patt=br"[^\S\n]+", tmp_dir=None, tmp_prefix="gxupload"): """ - Combines above methods: convert_newlines() and sep2tabs() - so that files do not need to be read twice + Converts newlines in a file to posix newlines and replaces spaces with tabs. >>> fname = get_test_fname('temp.txt') >>> with open(fname, 'wt') as fh: