Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Should dataInput of ClusterTree be named "taxa"? #1123

Open
MordorianGuy opened this issue Sep 6, 2023 · 2 comments
Open

Should dataInput of ClusterTree be named "taxa"? #1123

MordorianGuy opened this issue Sep 6, 2023 · 2 comments

Comments

@MordorianGuy
Copy link

final public Input<Alignment> dataInput = new Input<>("taxa", "alignment data used for calculating distances for clustering");

The "taxa" xml-input name confounds my user's eye since it takes Alignment objects. Should it be renamed to "data" or "alignment"?

P.S. One more point. By default, the BEAUti assigns the first partition as the input. It is not apparent and not expected behaviour. Maybe the unfiltered alignment would be better?

@MordorianGuy MordorianGuy changed the title Should dataInput of ClusterTree be named "taxa"? Should dataInput of ClusterTree be named **"taxa"**? Sep 6, 2023
@MordorianGuy MordorianGuy changed the title Should dataInput of ClusterTree be named **"taxa"**? Should dataInput of ClusterTree be named __"taxa"__? Sep 6, 2023
@MordorianGuy MordorianGuy changed the title Should dataInput of ClusterTree be named __"taxa"__? Should dataInput of ClusterTree be named "taxa"? Sep 6, 2023
@rbouckaert
Copy link
Member

@MordorianGuy you are right that data or alignment makes more sense -- however, for reasons of backward compatibility, it is not easy to change this. All BEAST v2.7.x compatible XML should still run on BEAST v2.7.y for y>x. There are a few more inconsistencies in Input names (e.g. start as capital in IsLabeledNewick in TreeParser, and these should be the same names: BeautiSubTemplate.suppressInputs BeautiConfig.suppressPlugins)

It can be achieved by adding an extra Input and deprecating the old one, or creating a new type of input that is known by more than one name (taxa and data for example), or perhaps some other mechanism for redirecting the Input values to the original Input. Not sure what the optimal solution is to remain both backward compatible and allow introduction of input name changes.

@MordorianGuy
Copy link
Author

@MordorianGuy you are right that data or alignment makes more sense -- however, for reasons of backward compatibility, it is not easy to change this. All BEAST v2.7.x compatible XML should still run on BEAST v2.7.y for y>x. There are a few more inconsistencies in Input names (e.g. start as capital in IsLabeledNewick in TreeParser, and these should be the same names: BeautiSubTemplate.suppressInputs BeautiConfig.suppressPlugins)

It can be achieved by adding an extra Input and deprecating the old one, or creating a new type of input that is known by more than one name (taxa and data for example), or perhaps some other mechanism for redirecting the Input values to the original Input. Not sure what the optimal solution is to remain both backward compatible and allow introduction of input name changes.

Thank you for your answer!

Yes, you are right: the backward compatibility is hard. I have thought more about something like regex switches between versions applying to parsed XML based on the version attribute of the beast tag. If we definitely know what we changed between specific versions, we may apply these changes consequently to adjust an old XML to new standards. At least if we talk about simple renaming.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants