Unify template validation #1043
Conversation
|
It looks like one of my previous PR introduced a potential stochastic failure in the system; since it is a test that relies in a dict order. I'm going to solve it as a separate PR and then I'll pull here. |
|
Changes Unknown when pulling 77afb1c on josenavas:unify-template-validation into * on biocore:master*. |
|
💥 ready for review! |
|
|
||
| # Check that we don't have duplicate columns | ||
| if len(set(md_template.columns)) != len(md_template.columns): | ||
| raise QiitaDBDuplicateHeaderError( |
There was a problem hiding this comment.
Hey, just noticed that this error message is not going to be very information (and it is not your fault, the code was like this already). If possible can you change to something along the lines of: 'The following header names are duplicated: %s' % ', '.join(find_duplicates(md_template.columns))
There was a problem hiding this comment.
Nix that, QiitaDBDuplicateHeaderError already takes care of that formatting.
|
Looks really good, just one comment, and if another person can have a look, this should be ready to be merged. |
|
Also, just noticed this has conflicts now, probably my bad :-\ |
|
Thanks @ElDeveloper , conflicts solved |
|
Thanks! This one needs one more reviewer and should be ready to go! It's a really quick one 🕐 |
|
ping @biocore/qiita-dev I would appreciate a review on all my PR, but specially this one. The changes are starting creating merge conflicts between them. |
| cls._check_subclass() | ||
| invalid_ids = get_invalid_sample_names(md_template.index) | ||
| if invalid_ids: | ||
| raise QiitaDBColumnError("The following sample names in the sample" |
There was a problem hiding this comment.
...in the template... (remove sample)
|
Couple questions/comments. |
|
@squirrelo addressed/commented |
Moves the function _clean_validate_template to the base class in order to remove code duplication. Tests added to both subclasses.