From 925e61e53a490adca3fb0938d8c7baf523e7d0ca Mon Sep 17 00:00:00 2001 From: Matthias Bernt Date: Wed, 21 Jul 2021 10:03:07 +0200 Subject: [PATCH 1/9] show that PairwiseDistanceMatrix sniffer mis-sniffs tabular data because its to unspecific --- lib/galaxy/datatypes/mothur.py | 3 +++ 1 file changed, 3 insertions(+) diff --git a/lib/galaxy/datatypes/mothur.py b/lib/galaxy/datatypes/mothur.py index 83dd8150f86..a9b8a21f619 100644 --- a/lib/galaxy/datatypes/mothur.py +++ b/lib/galaxy/datatypes/mothur.py @@ -491,6 +491,9 @@ class PairwiseDistanceMatrix(DistanceMatrix, Tabular): >>> fname = get_test_fname( 'mothur_datatypetest_false.mothur.pair.dist' ) >>> PairwiseDistanceMatrix().sniff( fname ) False + >>> fname = get_test_fname( '2.tabular' ) + >>> PairwiseDistanceMatrix().sniff( fname ) + False """ headers = iter_headers(file_prefix, sep='\t') count = 0 From 54abd6428a87e7ad642217f44d2de9829fae6b23 Mon Sep 17 00:00:00 2001 From: Matthias Bernt Date: Wed, 21 Jul 2021 10:48:36 +0200 Subject: [PATCH 2/9] disambiguate test data (2.tabular) --- lib/galaxy/datatypes/sniff.py | 2 +- lib/galaxy/datatypes/tabular.py | 2 +- lib/galaxy/datatypes/test/2.tabular | 4 +--- lib/galaxy/datatypes/test/test_tab2.tabular | 3 +++ 4 files changed, 6 insertions(+), 5 deletions(-) mode change 100644 => 120000 lib/galaxy/datatypes/test/2.tabular create mode 100644 lib/galaxy/datatypes/test/test_tab2.tabular diff --git a/lib/galaxy/datatypes/sniff.py b/lib/galaxy/datatypes/sniff.py index e67725ce893..c00a6ca9286 100644 --- a/lib/galaxy/datatypes/sniff.py +++ b/lib/galaxy/datatypes/sniff.py @@ -270,7 +270,7 @@ def guess_ext(fname, sniff_order, is_binary=False): >>> fname = get_test_fname('2.txt') >>> guess_ext(fname, sniff_order) 'txt' - >>> fname = get_test_fname('2.tabular') + >>> fname = get_test_fname('test_tab2.tabular') >>> guess_ext(fname, sniff_order) 'tabular' >>> fname = get_test_fname('3.txt') diff --git a/lib/galaxy/datatypes/tabular.py b/lib/galaxy/datatypes/tabular.py index 334098bcfda..147345e3688 100644 --- a/lib/galaxy/datatypes/tabular.py +++ b/lib/galaxy/datatypes/tabular.py @@ -686,7 +686,7 @@ class Pileup(Tabular): >>> fname = get_test_fname( '2.txt' ) >>> Pileup().sniff( fname ) # 2.txt False - >>> fname = get_test_fname( '2.tabular' ) + >>> fname = get_test_fname( 'test_tab2.tabular' ) >>> Pileup().sniff( fname ) False """ diff --git a/lib/galaxy/datatypes/test/2.tabular b/lib/galaxy/datatypes/test/2.tabular deleted file mode 100644 index d56c0bee10d..00000000000 --- a/lib/galaxy/datatypes/test/2.tabular +++ /dev/null @@ -1,3 +0,0 @@ -a 2 -c 1 -d 0 diff --git a/lib/galaxy/datatypes/test/2.tabular b/lib/galaxy/datatypes/test/2.tabular new file mode 120000 index 00000000000..2cae3b48c67 --- /dev/null +++ b/lib/galaxy/datatypes/test/2.tabular @@ -0,0 +1 @@ +../../../../test-data/2.tabular \ No newline at end of file diff --git a/lib/galaxy/datatypes/test/test_tab2.tabular b/lib/galaxy/datatypes/test/test_tab2.tabular new file mode 100644 index 00000000000..d56c0bee10d --- /dev/null +++ b/lib/galaxy/datatypes/test/test_tab2.tabular @@ -0,0 +1,3 @@ +a 2 +c 1 +d 0 From 0cffc65c265ef5ae4c4413103e19dffd7569feb4 Mon Sep 17 00:00:00 2001 From: Matthias Bernt Date: Mon, 2 Aug 2021 10:23:00 +0200 Subject: [PATCH 3/9] invert check to reduce indentation --- lib/galaxy/datatypes/mothur.py | 25 +++++++++++++------------ 1 file changed, 13 insertions(+), 12 deletions(-) diff --git a/lib/galaxy/datatypes/mothur.py b/lib/galaxy/datatypes/mothur.py index a9b8a21f619..a0c8ce5e070 100644 --- a/lib/galaxy/datatypes/mothur.py +++ b/lib/galaxy/datatypes/mothur.py @@ -498,20 +498,21 @@ class PairwiseDistanceMatrix(DistanceMatrix, Tabular): headers = iter_headers(file_prefix, sep='\t') count = 0 for line in headers: - if not line[0].startswith('@'): - if len(line) != 3: - return False + if line[0].startswith('@'): + continue + if len(line) != 3: + return False + try: + float(line[2]) try: - float(line[2]) - try: - # See if it's also an integer - int(line[2]) - except ValueError: - # At least one value is not an integer - all_ints = False + # See if it's also an integer + int(line[2]) except ValueError: - return False - count += 1 + # At least one value is not an integer + all_ints = False + except ValueError: + return False + count += 1 if count > 2: return not all_ints From 10ac67bb679f0f90459b631c36600e4c208446d9 Mon Sep 17 00:00:00 2001 From: Matthias Bernt Date: Mon, 2 Aug 2021 10:30:49 +0200 Subject: [PATCH 4/9] stricted sniffer for PairwiseDistanceMatrix --- lib/galaxy/datatypes/mothur.py | 13 +++++++++++++ 1 file changed, 13 insertions(+) diff --git a/lib/galaxy/datatypes/mothur.py b/lib/galaxy/datatypes/mothur.py index a0c8ce5e070..89e2aa9604d 100644 --- a/lib/galaxy/datatypes/mothur.py +++ b/lib/galaxy/datatypes/mothur.py @@ -502,6 +502,7 @@ class PairwiseDistanceMatrix(DistanceMatrix, Tabular): continue if len(line) != 3: return False + # check if col3 contains distances (floats) try: float(line[2]) try: @@ -513,10 +514,22 @@ class PairwiseDistanceMatrix(DistanceMatrix, Tabular): except ValueError: return False count += 1 + #check if col1 and col2 likely contain names + names = [False, False] + for c in [0, 1]: + try: + float(line[c]) + except ValueError: + names[c] = True + + if not names[0] or not names[1]: + return False if count > 2: return not all_ints + + return False From 3e7304bde0280743943320248419c67775182a6c Mon Sep 17 00:00:00 2001 From: M Bernt Date: Wed, 21 Jul 2021 20:46:46 +0200 Subject: [PATCH 5/9] Apply suggestions from code review Co-authored-by: Nicola Soranzo --- lib/galaxy/datatypes/mothur.py | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/lib/galaxy/datatypes/mothur.py b/lib/galaxy/datatypes/mothur.py index 89e2aa9604d..cd2f04f2956 100644 --- a/lib/galaxy/datatypes/mothur.py +++ b/lib/galaxy/datatypes/mothur.py @@ -488,11 +488,11 @@ class PairwiseDistanceMatrix(DistanceMatrix, Tabular): >>> fname = get_test_fname( 'mothur_datatypetest_true.mothur.pair.dist' ) >>> PairwiseDistanceMatrix().sniff( fname ) True - >>> fname = get_test_fname( 'mothur_datatypetest_false.mothur.pair.dist' ) - >>> PairwiseDistanceMatrix().sniff( fname ) + >>> fname = get_test_fname('mothur_datatypetest_false.mothur.pair.dist') + >>> PairwiseDistanceMatrix().sniff(fname) False - >>> fname = get_test_fname( '2.tabular' ) - >>> PairwiseDistanceMatrix().sniff( fname ) + >>> fname = get_test_fname('2.tabular') + >>> PairwiseDistanceMatrix().sniff(fname) False """ headers = iter_headers(file_prefix, sep='\t') From 77b6ef51f44380fe362ca4dd389a5e1ab968e4e3 Mon Sep 17 00:00:00 2001 From: Matthias Bernt Date: Mon, 2 Aug 2021 11:21:00 +0200 Subject: [PATCH 6/9] stricter test for PairwiseDistanceMatrix check if columns 1 and 2 contain at least one non-float (text) entry --- lib/galaxy/datatypes/mothur.py | 8 +++----- 1 file changed, 3 insertions(+), 5 deletions(-) diff --git a/lib/galaxy/datatypes/mothur.py b/lib/galaxy/datatypes/mothur.py index cd2f04f2956..5a709a7fc80 100644 --- a/lib/galaxy/datatypes/mothur.py +++ b/lib/galaxy/datatypes/mothur.py @@ -497,6 +497,7 @@ class PairwiseDistanceMatrix(DistanceMatrix, Tabular): """ headers = iter_headers(file_prefix, sep='\t') count = 0 + names = [False, False] for line in headers: if line[0].startswith('@'): continue @@ -514,22 +515,19 @@ class PairwiseDistanceMatrix(DistanceMatrix, Tabular): except ValueError: return False count += 1 - #check if col1 and col2 likely contain names - names = [False, False] + # check if col1 and col2 likely contain names for c in [0, 1]: try: float(line[c]) except ValueError: names[c] = True - + if not names[0] or not names[1]: return False if count > 2: return not all_ints - - return False From 67a1a35afedb18534698e0183f1ac75a430a7465 Mon Sep 17 00:00:00 2001 From: Matthias Bernt Date: Mon, 2 Aug 2021 12:13:09 +0200 Subject: [PATCH 7/9] stricter test for DistanceMatrix sniffer could be implemented for all, but maybe if done in doctests then to many sniff(filex) calls run multiple times. --- lib/galaxy/datatypes/mothur.py | 14 +++++--------- lib/galaxy/datatypes/sniff.py | 32 +++++++++++++++++++++++++------- 2 files changed, 30 insertions(+), 16 deletions(-) diff --git a/lib/galaxy/datatypes/mothur.py b/lib/galaxy/datatypes/mothur.py index 5a709a7fc80..27b1cdb6dc4 100644 --- a/lib/galaxy/datatypes/mothur.py +++ b/lib/galaxy/datatypes/mothur.py @@ -484,16 +484,12 @@ class PairwiseDistanceMatrix(DistanceMatrix, Tabular): Determines whether the file is a pairwise distance matrix (Column-formatted distance matrix) format The first and second columns have the sequence names and the third column is the distance between those sequences. - >>> from galaxy.datatypes.sniff import get_test_fname - >>> fname = get_test_fname( 'mothur_datatypetest_true.mothur.pair.dist' ) - >>> PairwiseDistanceMatrix().sniff( fname ) + >>> from galaxy.datatypes.sniff import get_test_iter + >>> pos, neg = get_test_iter(['mothur_datatypetest_true.mothur.pair.dist']) + >>> all([PairwiseDistanceMatrix().sniff(p) for p in pos]) + True + >>> all([not PairwiseDistanceMatrix().sniff(n) for n in neg]) True - >>> fname = get_test_fname('mothur_datatypetest_false.mothur.pair.dist') - >>> PairwiseDistanceMatrix().sniff(fname) - False - >>> fname = get_test_fname('2.tabular') - >>> PairwiseDistanceMatrix().sniff(fname) - False """ headers = iter_headers(file_prefix, sep='\t') count = 0 diff --git a/lib/galaxy/datatypes/sniff.py b/lib/galaxy/datatypes/sniff.py index c00a6ca9286..7cf725b1e7a 100644 --- a/lib/galaxy/datatypes/sniff.py +++ b/lib/galaxy/datatypes/sniff.py @@ -38,6 +38,24 @@ def get_test_fname(fname): return full_path +def get_test_iter(fnames): + """Returns test data paths separated into those that + should evaluate positive and negative given a list + of basenames that should evaluate positive. + """ + pos = set() + neg = set() + path, name = os.path.split(__file__) + path = os.path.join(path, 'test') + with os.scandir(path) as it: + for entry in it: + if entry.name in fnames: + pos.add(entry.path) + else: + neg.add(entry.path) + return pos, neg + + def sniff_with_cls(cls, fname): path = get_test_fname(fname) try: @@ -136,10 +154,11 @@ def convert_newlines_sep2tabs(fname, in_place=True, patt=br"[^\S\n]+", tmp_dir=N def iter_headers(fname_or_file_prefix, sep, count=60, comment_designator=None): idx = 0 - if isinstance(fname_or_file_prefix, FilePrefix): - file_iterator = fname_or_file_prefix.line_iterator() - else: - file_iterator = compression_utils.get_fileobj(fname_or_file_prefix) + if not isinstance(fname_or_file_prefix, FilePrefix): + fname_or_file_prefix = FilePrefix(fname_or_file_prefix) + if fname_or_file_prefix.binary: + return + file_iterator = fname_or_file_prefix.line_iterator() for line in file_iterator: line = line.rstrip('\n\r') if comment_designator is not None and comment_designator != '' and line.startswith(comment_designator): @@ -447,9 +466,8 @@ def guess_ext(fname, sniff_order, is_binary=False): # skip header check if data is already known to be binary if is_binary: return file_ext or 'binary' - try: - get_headers(file_prefix, None) - except UnicodeDecodeError: + # get_headers returns an empty list for binary (and empty) files + if len(get_headers(file_prefix, None)) == 0: return 'data' # default data type file extension if is_column_based(file_prefix, '\t', 1): return 'tabular' # default tabular data type file extension From 6c0fcab00cc9a3c1ba7cd7df59f34e0d7a76281f Mon Sep 17 00:00:00 2001 From: Matthias Bernt Date: Wed, 8 Sep 2021 18:16:55 +0200 Subject: [PATCH 8/9] revert some changes now included in https://github.com/galaxyproject/galaxy/pull/12418 --- lib/galaxy/datatypes/sniff.py | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/lib/galaxy/datatypes/sniff.py b/lib/galaxy/datatypes/sniff.py index 7cf725b1e7a..f0cc77fc96f 100644 --- a/lib/galaxy/datatypes/sniff.py +++ b/lib/galaxy/datatypes/sniff.py @@ -154,11 +154,10 @@ def convert_newlines_sep2tabs(fname, in_place=True, patt=br"[^\S\n]+", tmp_dir=N def iter_headers(fname_or_file_prefix, sep, count=60, comment_designator=None): idx = 0 - if not isinstance(fname_or_file_prefix, FilePrefix): - fname_or_file_prefix = FilePrefix(fname_or_file_prefix) - if fname_or_file_prefix.binary: - return - file_iterator = fname_or_file_prefix.line_iterator() + if isinstance(fname_or_file_prefix, FilePrefix): + file_iterator = fname_or_file_prefix.line_iterator() + else: + file_iterator = compression_utils.get_fileobj(fname_or_file_prefix) for line in file_iterator: line = line.rstrip('\n\r') if comment_designator is not None and comment_designator != '' and line.startswith(comment_designator): @@ -466,8 +465,9 @@ def guess_ext(fname, sniff_order, is_binary=False): # skip header check if data is already known to be binary if is_binary: return file_ext or 'binary' - # get_headers returns an empty list for binary (and empty) files - if len(get_headers(file_prefix, None)) == 0: + try: + get_headers(file_prefix, None) + except UnicodeDecodeError: return 'data' # default data type file extension if is_column_based(file_prefix, '\t', 1): return 'tabular' # default tabular data type file extension From e6d906f774e519ce5aed1d5903d7414a00bc58c6 Mon Sep 17 00:00:00 2001 From: Matthias Bernt Date: Mon, 13 Sep 2021 14:25:07 +0200 Subject: [PATCH 9/9] restore simple doc test --- lib/galaxy/datatypes/mothur.py | 11 ++++++----- lib/galaxy/datatypes/sniff.py | 18 ------------------ 2 files changed, 6 insertions(+), 23 deletions(-) diff --git a/lib/galaxy/datatypes/mothur.py b/lib/galaxy/datatypes/mothur.py index 27b1cdb6dc4..073d4f64499 100644 --- a/lib/galaxy/datatypes/mothur.py +++ b/lib/galaxy/datatypes/mothur.py @@ -484,12 +484,13 @@ class PairwiseDistanceMatrix(DistanceMatrix, Tabular): Determines whether the file is a pairwise distance matrix (Column-formatted distance matrix) format The first and second columns have the sequence names and the third column is the distance between those sequences. - >>> from galaxy.datatypes.sniff import get_test_iter - >>> pos, neg = get_test_iter(['mothur_datatypetest_true.mothur.pair.dist']) - >>> all([PairwiseDistanceMatrix().sniff(p) for p in pos]) - True - >>> all([not PairwiseDistanceMatrix().sniff(n) for n in neg]) + >>> from galaxy.datatypes.sniff import get_test_fname + >>> fname = get_test_fname( 'mothur_datatypetest_true.mothur.pair.dist' ) + >>> PairwiseDistanceMatrix().sniff( fname ) True + >>> fname = get_test_fname( 'mothur_datatypetest_false.mothur.pair.dist' ) + >>> PairwiseDistanceMatrix().sniff( fname ) + False """ headers = iter_headers(file_prefix, sep='\t') count = 0 diff --git a/lib/galaxy/datatypes/sniff.py b/lib/galaxy/datatypes/sniff.py index f0cc77fc96f..c00a6ca9286 100644 --- a/lib/galaxy/datatypes/sniff.py +++ b/lib/galaxy/datatypes/sniff.py @@ -38,24 +38,6 @@ def get_test_fname(fname): return full_path -def get_test_iter(fnames): - """Returns test data paths separated into those that - should evaluate positive and negative given a list - of basenames that should evaluate positive. - """ - pos = set() - neg = set() - path, name = os.path.split(__file__) - path = os.path.join(path, 'test') - with os.scandir(path) as it: - for entry in it: - if entry.name in fnames: - pos.add(entry.path) - else: - neg.add(entry.path) - return pos, neg - - def sniff_with_cls(cls, fname): path = get_test_fname(fname) try: