{"action":"created","issue":{"url":"https://api.github.com/repos/CompEvol/beast2/issues/43","labels_url":"https://api.github.com/repos/CompEvol/beast2/issues/43/labels{/name}","comments_url":"https://api.github.com/repos/CompEvol/beast2/issues/43/comments","events_url":"https://api.github.com/repos/CompEvol/beast2/issues/43/events","html_url":"https://github.com/CompEvol/beast2/issues/43","id":25874460,"number":43,"title":"Refactoring of TreeParser","user":{"login":"tgvaughan","id":512538,"avatar_url":"https://avatars.githubusercontent.com/u/512538?v=3","gravatar_id":"","url":"https://api.github.com/users/tgvaughan","html_url":"https://github.com/tgvaughan","followers_url":"https://api.github.com/users/tgvaughan/followers","following_url":"https://api.github.com/users/tgvaughan/following{/other_user}","gists_url":"https://api.github.com/users/tgvaughan/gists{/gist_id}","starred_url":"https://api.github.com/users/tgvaughan/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/tgvaughan/subscriptions","organizations_url":"https://api.github.com/users/tgvaughan/orgs","repos_url":"https://api.github.com/users/tgvaughan/repos","events_url":"https://api.github.com/users/tgvaughan/events{/privacy}","received_events_url":"https://api.github.com/users/tgvaughan/received_events","type":"User","site_admin":false},"labels":[{"url":"https://api.github.com/repos/CompEvol/beast2/labels/enhancement","name":"enhancement","color":"0052cc"},{"url":"https://api.github.com/repos/CompEvol/beast2/labels/HIGH%20priority","name":"HIGH priority","color":"eb6420"}],"state":"open","locked":false,"assignee":null,"milestone":null,"comments":1,"created_at":"2014-01-19T04:45:14Z","updated_at":"2015-03-23T21:49:13Z","closed_at":null,"body":"TreeParser is rather too complex and has a very large number of interacting Inputs. It should be rationalised, perhaps by introducing a subclass. One key distinction is whether the labels in the Newick tree are considered to be node numbers of labels. For example, this could be separated into a superclass that only handles numbers and a subclass that handles labels.\r\n\r\nTreeParser should also have an option for single-child nodes of either reading them in as single-child nodes (i.e. for purposes of beast-graphics or TreeStat2) or as two-child nodes with a zero branch to labelled child (i.e. for BEAST MCMC of sampled ancestor trees)."},"comment":{"url":"https://api.github.com/repos/CompEvol/beast2/issues/comments/85212831","html_url":"https://github.com/CompEvol/beast2/issues/43#issuecomment-85212831","issue_url":"https://api.github.com/repos/CompEvol/beast2/issues/43","id":85212831,"user":{"login":"tgvaughan","id":512538,"avatar_url":"https://avatars.githubusercontent.com/u/512538?v=3","gravatar_id":"","url":"https://api.github.com/users/tgvaughan","html_url":"https://github.com/tgvaughan","followers_url":"https://api.github.com/users/tgvaughan/followers","following_url":"https://api.github.com/users/tgvaughan/following{/other_user}","gists_url":"https://api.github.com/users/tgvaughan/gists{/gist_id}","starred_url":"https://api.github.com/users/tgvaughan/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/tgvaughan/subscriptions","organizations_url":"https://api.github.com/users/tgvaughan/orgs","repos_url":"https://api.github.com/users/tgvaughan/repos","events_url":"https://api.github.com/users/tgvaughan/events{/privacy}","received_events_url":"https://api.github.com/users/tgvaughan/received_events","type":"User","site_admin":false},"created_at":"2015-03-23T21:49:13Z","updated_at":"2015-03-23T21:49:13Z","body":"Anyone interested in simplifying TreeParser might want to take a squiz at the above commit.  I know we've discussed the drawbacks of adding dependencies to beast before (the above requires a 370kb jar be added to lib/), but the advantages are:\r\n\r\n* Parser generated from human-readable grammar\r\n* Robust - e.g. no accidental parsing of additional close parens.\r\n* Easy to modify.\r\n\r\nThe above commit (c7795cd) replaces only the parseNewick() method.  I think some of the other code could also be simplified by letting the parser handle things like meta-data strings.\r\n\r\nI wouldn't at all suggest merging this commit with the master branch btw - it passes the unit tests, but there are enough things around that rely on details of the existing implementation that this sketch no-doubt misses."}}