-
Notifications
You must be signed in to change notification settings - Fork 40
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
ENH: Improved alpha diversity format validation #204
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Minor comments
q2_types/sample_data/_format.py
Outdated
return i > 1 | ||
header = None | ||
records_seen = 0 | ||
file_ = enumerate(fh) if level == 'min' else zip(range(10), fh) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
file_ = enumerate(fh) if level == 'min' else zip(range(10), fh) | |
file_ = enumerate(fh, 1) if level == 'min' else zip(range(1, 10), fh) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This line becomes 81 cols long (which was why I didn't do this in the first place). Options: promote from a ternary to a real if/else block; unindent the whole block by removing the filehandle context manager and instead manually closing. Any other options come to mind?
q2_types/sample_data/_format.py
Outdated
records_seen = 0 | ||
file_ = enumerate(fh) if level == 'min' else zip(range(10), fh) | ||
for i, line in file_: | ||
i = i + 1 # For easier reporting |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
i = i + 1 # For easier reporting |
q2_types/sample_data/_format.py
Outdated
continue # Blank line | ||
elif line.startswith('#'): | ||
continue # Comment line | ||
cells = [c.strip() for c in line.rstrip('\n').split('\t')] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Let's switch this to csv
for quote handling
format.validate() | ||
|
||
def test_alpha_diversity_format_validate_negative(self): |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
:)
Refactors the existing
sniff
er to use the current validation API. Also, removes the (likely accidental off-by-one) restriction that a minimum of two samples are present (it looks like the original code forgot to account for the header row).