From 97c221cf53abc9c979871f4acc9509a0f253297e Mon Sep 17 00:00:00 2001 From: Gaurav Vaidya Date: Wed, 30 Aug 2023 00:27:09 -0400 Subject: [PATCH 1/6] Added test for Newick strings that have quotes them them. This is related to https://github.com/veg/phylotree.js/issues/438 --- test/data/newick-with-strings.new | 1 + test/formats-test.js | 13 +++++++++++++ 2 files changed, 14 insertions(+) create mode 100644 test/data/newick-with-strings.new diff --git a/test/data/newick-with-strings.new b/test/data/newick-with-strings.new new file mode 100644 index 0000000..aeec6c4 --- /dev/null +++ b/test/data/newick-with-strings.new @@ -0,0 +1 @@ +('Alpha beta', ('Alpha gamma', 'Delta''s epsilon')) diff --git a/test/formats-test.js b/test/formats-test.js index 12e8483..2d54a6f 100644 --- a/test/formats-test.js +++ b/test/formats-test.js @@ -74,3 +74,16 @@ tape("BEAST newick parse", function(test) { test.assert(almost_equal(beast_data.b_u_N_range[1], 10.0)); test.end(); }); + +tape("Handle Newick strings with spaces", function(test) { + let nwk = fs.readFileSync(__dirname + "/data/newick-with-strings.new").toString(); + let phylo = new phylotree.phylotree(nwk); + + let test_leaves = ['Alpha beta', 'Alpha gamma', "Delta's epsilon"]; + test_leaves.forEach(function(leaf) { + let node = phylo.getNodeByName(leaf); + test.equal(node.data.name, leaf); + }); + + test.equal(phylo.getNewick(), "()"); +}); From 03c5b398777dafe233642d26790f31b8a4e9b75f Mon Sep 17 00:00:00 2001 From: Gaurav Vaidya Date: Wed, 30 Aug 2023 00:51:31 -0400 Subject: [PATCH 2/6] Improved test. --- test/data/newick-with-strings.new | 1 - test/formats-test.js | 15 +++++++++++++-- 2 files changed, 13 insertions(+), 3 deletions(-) delete mode 100644 test/data/newick-with-strings.new diff --git a/test/data/newick-with-strings.new b/test/data/newick-with-strings.new deleted file mode 100644 index aeec6c4..0000000 --- a/test/data/newick-with-strings.new +++ /dev/null @@ -1 +0,0 @@ -('Alpha beta', ('Alpha gamma', 'Delta''s epsilon')) diff --git a/test/formats-test.js b/test/formats-test.js index 2d54a6f..33d0a01 100644 --- a/test/formats-test.js +++ b/test/formats-test.js @@ -76,7 +76,13 @@ tape("BEAST newick parse", function(test) { }); tape("Handle Newick strings with spaces", function(test) { - let nwk = fs.readFileSync(__dirname + "/data/newick-with-strings.new").toString(); + /* + * As per the Newick specification, labels can be quoted with single quote characters, + * which can be escaped with double single quotes. This test ensures that phylotree.js + * can read and write Newick strings correctly. + */ + + let nwk = "('Alpha beta', ('Alpha gamma', 'Delta''s epsilon'))"; let phylo = new phylotree.phylotree(nwk); let test_leaves = ['Alpha beta', 'Alpha gamma', "Delta's epsilon"]; @@ -85,5 +91,10 @@ tape("Handle Newick strings with spaces", function(test) { test.equal(node.data.name, leaf); }); - test.equal(phylo.getNewick(), "()"); + // This would be identical to the original Newick string, but phylotree.js coerces + // lengths to 1 (see https://github.com/veg/phylotree.js/issues/440). So we expect + // all lengths to be set to 1 on export. + // + // This test currently fails because of https://github.com/veg/phylotree.js/issues/438 + test.equal(phylo.getNewick(), "('Alpha beta':1, ('Alpha gamma':1, 'Delta''s epsilon':1))"); }); From 2ef78785f62f9a73367243b19275cb2fa5deea1b Mon Sep 17 00:00:00 2001 From: Gaurav Vaidya Date: Wed, 30 Aug 2023 00:55:37 -0400 Subject: [PATCH 3/6] First stab at fixing Newick single quote string issue. --- src/formats/newick.js | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/src/formats/newick.js b/src/formats/newick.js index 831b890..8afe873 100644 --- a/src/formats/newick.js +++ b/src/formats/newick.js @@ -250,6 +250,10 @@ export function getNewick(annotator, root) { } if(n.data.name != 'root') { + const node_label = n.data.name.replace("'", "''"); + if (node_label.contains(/\w/)) { + element_array.push("'" + node_label + "'"); + } element_array.push(n.data.name); } element_array.push(annotator(n)); From cad10902dbc7b7466a29fb70b5b1db5dda27c237 Mon Sep 17 00:00:00 2001 From: Gaurav Vaidya Date: Wed, 30 Aug 2023 00:59:19 -0400 Subject: [PATCH 4/6] Improved code. --- src/formats/newick.js | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/src/formats/newick.js b/src/formats/newick.js index 8afe873..b8a438d 100644 --- a/src/formats/newick.js +++ b/src/formats/newick.js @@ -251,10 +251,13 @@ export function getNewick(annotator, root) { if(n.data.name != 'root') { const node_label = n.data.name.replace("'", "''"); - if (node_label.contains(/\w/)) { + + // Escape the entire string if it contains any whitespace. + if (/\w/.test(node_label)) { element_array.push("'" + node_label + "'"); + } else { + element_array.push(n.data.name); } - element_array.push(n.data.name); } element_array.push(annotator(n)); From 0b6b45167aa55aad2e641ee880712fea2cf4c50d Mon Sep 17 00:00:00 2001 From: Gaurav Vaidya Date: Wed, 30 Aug 2023 00:59:52 -0400 Subject: [PATCH 5/6] Improved test. --- test/formats-test.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/formats-test.js b/test/formats-test.js index 33d0a01..0b7ce96 100644 --- a/test/formats-test.js +++ b/test/formats-test.js @@ -96,5 +96,5 @@ tape("Handle Newick strings with spaces", function(test) { // all lengths to be set to 1 on export. // // This test currently fails because of https://github.com/veg/phylotree.js/issues/438 - test.equal(phylo.getNewick(), "('Alpha beta':1, ('Alpha gamma':1, 'Delta''s epsilon':1))"); + test.equal(phylo.getNewick(), "('Alpha beta':1,('Alpha gamma':1,'Delta''s epsilon':1):1):1"); }); From 6341cee3534af618149c9b64600cd5ee3b431639 Mon Sep 17 00:00:00 2001 From: Gaurav Vaidya Date: Wed, 30 Aug 2023 01:00:53 -0400 Subject: [PATCH 6/6] Improved test. --- test/formats-test.js | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/test/formats-test.js b/test/formats-test.js index 0b7ce96..df585e4 100644 --- a/test/formats-test.js +++ b/test/formats-test.js @@ -94,7 +94,7 @@ tape("Handle Newick strings with spaces", function(test) { // This would be identical to the original Newick string, but phylotree.js coerces // lengths to 1 (see https://github.com/veg/phylotree.js/issues/440). So we expect // all lengths to be set to 1 on export. - // - // This test currently fails because of https://github.com/veg/phylotree.js/issues/438 - test.equal(phylo.getNewick(), "('Alpha beta':1,('Alpha gamma':1,'Delta''s epsilon':1):1):1"); + test.equal(phylo.getNewick(), "('Alpha beta':1,('Alpha gamma':1,'Delta''s epsilon':1):1):1;"); + + test.end(); });