-
Notifications
You must be signed in to change notification settings - Fork 38
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
ISA Validation Configurations Too Restrictive #520
Comments
@ptth222 you are right about the missing documentation about this step, something we are working at addressing. It turns out that the match of those strings and any departure from the expected value will cause a validation error, which does not return a very helpful message. If you have a validation failure, it is either because there is mismatch on those values, or an entirely new assay type has been defined locally but is absent from the ISA configuration supplied to the ISA validator or from the default one. To your point about the purpose of this behaviour, it was somehow to use the ISA configuration to "enforce" so called "Minimal Information Requirement" defined by a community for a technology domain. For instance, the ISA configurations supporting NGS applications are aligned to INSDC SRA specification. Degrading an ISA configuration to simplify it would result in failure to convert to SRA xml down the line.
Coordination the development and maintenance of all these requires resources but if there are needs in your area, please let us know and let's have a follow-up (zoom call) atb |
Thank you for the reply. I think I understand a little better. We have something similar built into MESSES, but it is strictly optional. If the user wants to validate more specific requirements then there are ways to do it, but we they have to opt into it. We don't have any defaults built into the package, but we do provide examples that users can start from or go by. I like that you have put additional validation capabilities in, but I would have preferred them to be more optional and targeted. I am going to enumerate some suggestions here:
I don't think any of these harm your intent, and would make things easier for the user. I wouldn't mind attempting some of this in a PR, but I would prefer to have some confirmation that these are okay and hash out any specifics if necessary. These changes wouldn't be as small as most of the PRs I have submitted so far. |
@ptth222 all great suggestions/observations and thank you for offering contribution. best of organise a call to agree on scope. much appreciated |
Hunter's schedule is a little hectic right now, but we could meet at either of these times this week: These are in our time, Eastern Standard. I'm not sure what that translates to for you. If those aren't great, maybe you can suggest a time or we might have to try for the week after. |
@ptth222 apologies for the belated reply. unfortunately, Jan 30 & 31 are no go for us. i'll follow with a doodle now. |
isajson.validate() has a "config_dir" parameter that defaults to using some JSON configuration files for various technology and measurement types, but if you don't have one of those types, errors are reported. Is the intention to force users to add configurations to validate new experiment types? In general these seem like extra validations that aren't necessary to validate whether something fits the ISA format since they validate specific word choice and protocol order, but there is no way to indicate to the function not to do these checks. My expectation would be that setting config_dir to None would disable this highly specific validation.
I don't see any documentation on needing to create these configuration files or how they are structured, anything at all on them. Am I looking in the wrong place?
For isajson.validate() it looks fairly straight forward to change the code so it would not run these validations if config_dir was None. isatab.validate() has the same problem though, and it would be much more difficult to try and offer a way to not do them. This makes it seem to me that it was intended to force the user to create configuration files, but then the lack of documentation on it contradicts that idea. What is the intention with these configuration files for users with new experiment types?
The text was updated successfully, but these errors were encountered: