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

Change Newick validation from sniff to validate #247

Open
ChrisKeefe opened this issue Jun 10, 2020 · 1 comment
Open

Change Newick validation from sniff to validate #247

ChrisKeefe opened this issue Jun 10, 2020 · 1 comment

Comments

@ChrisKeefe
Copy link
Contributor

ChrisKeefe commented Jun 10, 2020

Improvement Description
Proper validation, possibly including empty-tree detection, could better protect users from passing bad phylogenies.

Current Behavior
NewickFormat uses a sniff pulled from skbio. If this is the sniffer linked in References, it catches empty trees, but only checks 100 tokens.

Proposed Behavior
A more comprehensive validation.

Questions

  1. Is the skbio sniffer already doing what we need?
  2. Is a full-length validation reasonable, timewise?

References

  1. This might be our sniffer?
  2. Newick overview
@nbokulich
Copy link
Member

Is the skbio sniffer already doing what we need?

In this same vein, I wonder if the validator should also check that all branches have a length.

This error sometimes crops up on the forum, e.g., when a tree is imported from outside (including some reference phylogenies!)

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