From d24933b88eae9560f5fffdbd5881e0bb39b6c785 Mon Sep 17 00:00:00 2001 From: Kartik Mohta Date: Tue, 29 Aug 2017 02:05:04 -0400 Subject: [PATCH 1/9] Small fixes to make it work with Python 3 Note that two tests fail due to internal changes in Python 3: - test_iterable_literals_eval: The order of keys in dict changed in Py3 - test_no_evaluation: ast.literal_eval('5 -2') gives 3 in Py3 --- src/xacro/__init__.py | 2 +- test/test_xacro.py | 9 ++++++--- xacro.py | 2 +- 3 files changed, 8 insertions(+), 5 deletions(-) diff --git a/src/xacro/__init__.py b/src/xacro/__init__.py index 0c152230..e171cf7a 100644 --- a/src/xacro/__init__.py +++ b/src/xacro/__init__.py @@ -950,7 +950,7 @@ def process_doc(doc, if do_check_order and symbols.redefined: warning("Document is incompatible to --inorder processing.") warning("The following properties were redefined after usage:") - for k, v in symbols.redefined.iteritems(): + for k, v in symbols.redefined.items(): message(k, "redefined in", v, color='yellow') diff --git a/test/test_xacro.py b/test/test_xacro.py index 450d456a..61a6b235 100644 --- a/test/test_xacro.py +++ b/test/test_xacro.py @@ -12,7 +12,10 @@ import shutil import subprocess import re -from cStringIO import StringIO +try: + from cStringIO import StringIO # Python 2.x +except ImportError: + from io import StringIO # Python 3.x from contextlib import contextmanager @@ -167,8 +170,8 @@ def test_is_valid_name(self): def test_resolve_macro(self): # define three nested macro dicts with the same macro names (keys) content = {'xacro:simple': 'simple'} - ns2 = dict({k: v+'2' for k,v in content.iteritems()}) - ns1 = dict({k: v+'1' for k,v in content.iteritems()}) + ns2 = dict({k: v+'2' for k,v in content.items()}) + ns1 = dict({k: v+'1' for k,v in content.items()}) ns1.update(ns2=ns2) macros = dict(content) macros.update(ns1=ns1) diff --git a/xacro.py b/xacro.py index f162aff3..e6d1e10f 100755 --- a/xacro.py +++ b/xacro.py @@ -50,7 +50,7 @@ this_dir_cwd = os.getcwd() os.chdir(cur_dir) # Remove this dir from path -sys.path = filter(lambda a: a not in [this_dir, this_dir_cwd], sys.path) +sys.path = [a for a in sys.path if a not in [this_dir, this_dir_cwd]] import xacro from xacro.color import warning From 4f65a2e128dfa4f10e968f45ca304dd24c764f7a Mon Sep 17 00:00:00 2001 From: Kartik Mohta Date: Tue, 12 Sep 2017 23:53:46 -0400 Subject: [PATCH 2/9] Add xml.dom.minidom import --- src/xacro/xmlutils.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/xacro/xmlutils.py b/src/xacro/xmlutils.py index fe310143..df891097 100644 --- a/src/xacro/xmlutils.py +++ b/src/xacro/xmlutils.py @@ -30,7 +30,7 @@ # Authors: Stuart Glaser, William Woodall, Robert Haschke # Maintainer: Morgan Quigley -import xml +import xml.dom.minidom from .color import warning def first_child_element(elt): From 4196b18b3e8e3c0bc772f2a40112bc5621c382f7 Mon Sep 17 00:00:00 2001 From: Robert Haschke Date: Thu, 14 Sep 2017 13:50:22 +0200 Subject: [PATCH 3/9] minor performance improvement avoid calling lex.peek() multiple times --- src/xacro/__init__.py | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/src/xacro/__init__.py b/src/xacro/__init__.py index e171cf7a..c010cce4 100644 --- a/src/xacro/__init__.py +++ b/src/xacro/__init__.py @@ -600,13 +600,14 @@ def handle_extension(s): lex = QuickLexer(LEXER) lex.lex(text) while lex.peek(): - if lex.peek()[0] == lex.EXPR: + id = lex.peek()[0] + if id == lex.EXPR: results.append(handle_expr(lex.next()[1][2:-1])) - elif lex.peek()[0] == lex.EXTENSION: + elif id == lex.EXTENSION: results.append(handle_extension(lex.next()[1][2:-1])) - elif lex.peek()[0] == lex.TEXT: + elif id == lex.TEXT: results.append(lex.next()[1]) - elif lex.peek()[0] == lex.DOLLAR_DOLLAR_BRACE: + elif id == lex.DOLLAR_DOLLAR_BRACE: results.append(lex.next()[1][1:]) # return single element as is, i.e. typed if len(results) == 1: From f5367e9dd2391c8e7f733604aadbabbc81b5915f Mon Sep 17 00:00:00 2001 From: Robert Haschke Date: Wed, 13 Sep 2017 10:05:14 +0200 Subject: [PATCH 4/9] fixed literal parsing Parsing of number and boolean literals was done with ast.literal_eval(). However, since python3 this function evaluates expressions like "5 -3" as the number result and not a string anymore. Hence resort to explicit type-casting to int, float, or boolean. --- src/xacro/__init__.py | 24 ++++++++++-------------- 1 file changed, 10 insertions(+), 14 deletions(-) diff --git a/src/xacro/__init__.py b/src/xacro/__init__.py index c010cce4..42ba00fc 100644 --- a/src/xacro/__init__.py +++ b/src/xacro/__init__.py @@ -203,17 +203,13 @@ def __init__(self, parent=None): @staticmethod def _eval_literal(value): if isinstance(value, _basestr): - try: - # try to evaluate as literal, e.g. number, boolean, etc. - # this is needed to handle numbers in property definitions as numbers, not strings - evaluated = ast.literal_eval(value.strip()) - # However, (simple) list, tuple, dict expressions will be evaluated as such too, - # which would break expected behaviour. Thus we only accept the evaluation otherwise. - if not isinstance(evaluated, (list, dict, tuple)): - return evaluated - except: - pass - + # try to evaluate as number literal or boolean + # this is needed to handle numbers in property definitions as numbers, not strings + for f in [int, float, lambda x: get_boolean_value(x, None)]: # order of types is important! + try: + return f(value) + except: + pass return value def _resolve_(self, key): @@ -751,9 +747,9 @@ def get_boolean_value(value, condition): """ try: if isinstance(value, _basestr): - if value == 'true': return True - elif value == 'false': return False - else: return ast.literal_eval(value) + if value == 'true' or value == 'True': return True + elif value == 'false' or value == 'False': return False + else: return bool(int(value)) else: return bool(value) except: From c53275cad5d8b523d7800081458d562ba83ff4fe Mon Sep 17 00:00:00 2001 From: Robert Haschke Date: Thu, 14 Sep 2017 14:28:13 +0200 Subject: [PATCH 5/9] more pythonic equality test --- test/test_xacro.py | 18 +++++++++--------- 1 file changed, 9 insertions(+), 9 deletions(-) diff --git a/test/test_xacro.py b/test/test_xacro.py index 61a6b235..2235d14c 100644 --- a/test/test_xacro.py +++ b/test/test_xacro.py @@ -26,22 +26,22 @@ def all_attributes_match(a, b): if len(a.attributes) != len(b.attributes): print("Different number of attributes") return False - a_atts = [(a.attributes.item(i).name, a.attributes.item(i).value) for i in range(len(a.attributes))] - b_atts = [(b.attributes.item(i).name, b.attributes.item(i).value) for i in range(len(b.attributes))] + a_atts = a.attributes.items() + b_atts = b.attributes.items() a_atts.sort() b_atts.sort() - for i in range(len(a_atts)): - if a_atts[i][0] != b_atts[i][0]: - print("Different attribute names: %s and %s" % (a_atts[i][0], b_atts[i][0])) + for a, b in zip(a_atts, b_atts): + if a[0] != b[0]: + print("Different attribute names: %s and %s" % (a[0], b[0])) return False try: - if abs(float(a_atts[i][1]) - float(b_atts[i][1])) > 1.0e-9: - print("Different attribute values: %s and %s" % (a_atts[i][1], b_atts[i][1])) + if abs(float(a[1]) - float(b[1])) > 1.0e-9: + print("Different attribute values: %s and %s" % (a[1], b[1])) return False except ValueError: # Attribute values aren't numeric - if a_atts[i][1] != b_atts[i][1]: - print("Different attribute values: %s and %s" % (a_atts[i][1], b_atts[i][1])) + if a[1] != b[1]: + print("Different attribute values: %s and %s" % (a[1], b[1])) return False return True From c2a17105d5f468806690f5914a7058ee4d5bdc58 Mon Sep 17 00:00:00 2001 From: Robert Haschke Date: Wed, 13 Sep 2017 00:41:05 +0200 Subject: [PATCH 6/9] improve matching test for text - handle numeric values at arbitrary locations, e.g. "3.1415 0 1" - ignore dict order, correctly parsing dict expressions and comparing resulting dicts --- test/test_xacro.py | 57 ++++++++++++++++++++++++++++++++++++---------- 1 file changed, 45 insertions(+), 12 deletions(-) diff --git a/test/test_xacro.py b/test/test_xacro.py index 2235d14c..41e11217 100644 --- a/test/test_xacro.py +++ b/test/test_xacro.py @@ -12,6 +12,7 @@ import shutil import subprocess import re +import ast try: from cStringIO import StringIO # Python 2.x except ImportError: @@ -22,6 +23,34 @@ # regex to match whitespace whitespace = re.compile(r'\s+') +def text_values_match(a, b): + # generic comparison + if whitespace.sub(' ', a).strip() == whitespace.sub(' ', b).strip(): + return True + + try: # special handling of dicts: ignore order + a_dict = ast.literal_eval(a) + b_dict = ast.literal_eval(b) + if (isinstance(a_dict, dict) and isinstance(b_dict, dict) and a_dict == b_dict): + return True + except: # Attribute values aren't dicts + pass + + # on failure, try to split a and b at whitespace and compare snippets + def match_splits(a_, b_): + if len(a_) != len(b_): return False + for a, b in zip(a_, b_): + if a == b: continue + try: # compare numeric values only up to some accuracy + if abs(float(a) - float(b)) > 1.0e-9: + return False + except ValueError: # values aren't numeric and not identical + return False + return True + + return match_splits(a.split(), b.split()) + + def all_attributes_match(a, b): if len(a.attributes) != len(b.attributes): print("Different number of attributes") @@ -35,24 +64,18 @@ def all_attributes_match(a, b): if a[0] != b[0]: print("Different attribute names: %s and %s" % (a[0], b[0])) return False - try: - if abs(float(a[1]) - float(b[1])) > 1.0e-9: - print("Different attribute values: %s and %s" % (a[1], b[1])) - return False - except ValueError: # Attribute values aren't numeric - if a[1] != b[1]: - print("Different attribute values: %s and %s" % (a[1], b[1])) - return False - + if not text_values_match(a[1], b[1]): + print("Different attribute values: %s and %s" % (a[1], b[1])) + return False return True + def text_matches(a, b): - a_norm = whitespace.sub(' ', a) - b_norm = whitespace.sub(' ', b) - if a_norm.strip() == b_norm.strip(): return True + if text_values_match(a, b): return True print("Different text values: '%s' and '%s'" % (a, b)) return False + def nodes_match(a, b, ignore_nodes): if not a and not b: return True @@ -141,6 +164,16 @@ def test_normalize_whitespace_text(self): def test_normalize_whitespace_trim(self): self.assertTrue(text_matches(" foo bar ", "foo \t\n\r bar")) + def test_match_similar_numbers(self): + self.assertTrue(text_matches("0.123456789", "0.123456788")) + def test_mismatch_different_numbers(self): + self.assertFalse(text_matches("0.123456789", "0.1234567879")) + + def test_match_unordered_dicts(self): + self.assertTrue(text_matches("{'a': 1, 'b': 2, 'c': 3}", "{'c': 3, 'b': 2, 'a': 1}")) + def test_mismatch_different_dicts(self): + self.assertFalse(text_matches("{'a': 1, 'b': 2, 'c': 3}", "{'c': 3, 'b': 2, 'a': 0}")) + def test_empty_node_vs_whitespace(self): self.assertTrue(xml_matches('''''', ''' \t\n\r ''')) def test_whitespace_vs_empty_node(self): From 1faf065e0c6687fb6bf98f9ce0004e2896da7b93 Mon Sep 17 00:00:00 2001 From: Robert Haschke Date: Fri, 15 Sep 2017 08:45:58 +0200 Subject: [PATCH 7/9] enable Travis for python 3 --- .travis.yml | 1 + 1 file changed, 1 insertion(+) diff --git a/.travis.yml b/.travis.yml index 5b73eb17..7cd5e1cd 100644 --- a/.travis.yml +++ b/.travis.yml @@ -3,6 +3,7 @@ sudo: enabled language: python python: - "2.7" + - "3.6" # command to install dependencies install: From fb41970ceda6b189bc902f25431ff8a44cf43bc6 Mon Sep 17 00:00:00 2001 From: Robert Haschke Date: Fri, 15 Sep 2017 08:46:14 +0200 Subject: [PATCH 8/9] open issue on python 3.x < 3.6 --- src/xacro/__init__.py | 3 +++ 1 file changed, 3 insertions(+) diff --git a/src/xacro/__init__.py b/src/xacro/__init__.py index 42ba00fc..d2264117 100644 --- a/src/xacro/__init__.py +++ b/src/xacro/__init__.py @@ -573,6 +573,9 @@ def grab_properties(elt, table): elt = next +# TODO: The order of arguments here is relevant! +# However, python 3.0 - 3.5 doesn't preserve order of keyword arguments +# This is only fixed with https://www.python.org/dev/peps/pep-0468 in 3.6 LEXER = QuickLexer(DOLLAR_DOLLAR_BRACE=r"\$\$+\{", EXPR=r"\$\{[^\}]*\}", EXTENSION=r"\$\([^\)]*\)", From ff574c91ca62b11fd4c82b6c18dfd5b945ed2963 Mon Sep 17 00:00:00 2001 From: Robert Haschke Date: Fri, 15 Sep 2017 09:56:06 +0200 Subject: [PATCH 9/9] improved lexer's regular expressions - made them mutually exclusive, such that we don't face the keyword args ordering issue anymore - fixed some unhandled corner cases (added appropriate test cases as well) - handle $$( additionally to $${ - handle text in front of $$ or single $ --- .travis.yml | 1 + src/xacro/__init__.py | 11 ++++------- test/test_xacro.py | 8 ++++---- 3 files changed, 9 insertions(+), 11 deletions(-) diff --git a/.travis.yml b/.travis.yml index 7cd5e1cd..815c851e 100644 --- a/.travis.yml +++ b/.travis.yml @@ -3,6 +3,7 @@ sudo: enabled language: python python: - "2.7" + - "3.5" - "3.6" # command to install dependencies diff --git a/src/xacro/__init__.py b/src/xacro/__init__.py index d2264117..633f7f50 100644 --- a/src/xacro/__init__.py +++ b/src/xacro/__init__.py @@ -573,13 +573,10 @@ def grab_properties(elt, table): elt = next -# TODO: The order of arguments here is relevant! -# However, python 3.0 - 3.5 doesn't preserve order of keyword arguments -# This is only fixed with https://www.python.org/dev/peps/pep-0468 in 3.6 -LEXER = QuickLexer(DOLLAR_DOLLAR_BRACE=r"\$\$+\{", - EXPR=r"\$\{[^\}]*\}", - EXTENSION=r"\$\([^\)]*\)", - TEXT=r"([^\$]|\$[^{(]|\$$)+") +LEXER = QuickLexer(DOLLAR_DOLLAR_BRACE=r"^\$\$+(\{|\()", # multiple $ in a row, followed by { or ( + EXPR=r"^\$\{[^\}]*\}", # stuff starting with ${ + EXTENSION=r"^\$\([^\)]*\)", # stuff starting with $( + TEXT=r"[^$]+|\$[^{($]+|\$$") # any text w/o $ or $ following any chars except {($ or single $ # evaluate text and return typed value diff --git a/test/test_xacro.py b/test/test_xacro.py index 41e11217..0fa8a726 100644 --- a/test/test_xacro.py +++ b/test/test_xacro.py @@ -447,13 +447,13 @@ def test_substitution_args_arg(self): def test_escaping_dollar_braces(self): self.assert_matches( - self.quick_xacro(''''''), - '''''') + self.quick_xacro(''''''), + '''''') def test_just_a_dollar_sign(self): self.assert_matches( - self.quick_xacro(''''''), - '''''') + self.quick_xacro(''''''), + '''''') def test_multiple_insert_blocks(self): self.assert_matches(