ISA-tools / isa-api

ISA tools API
https://isa-tools.org
Other
40 stars 38 forks source link

Validate config changes #552

Open ptth222 opened 6 months ago

ptth222 commented 6 months ago

This PR goes all the way back to the initial meeting we had to discuss adding a way to opt out of the configuration validations. I have implemented that here. A "no_config" parameter was added to both the JSON and Tab validation and propagated appropriately so configuration validation can be optional. There are also a few messaging changes and some slight fixes.

One issue I found during testing was that some of the rules in the Tab validate have lines like result = result and func(). This essentially makes it to where the func() won't run after 1 error or warning is found. This eliminates some warnings or errors from being repeated multiple times, but also hides when there are multiple different ones at the same time. "test_b_ii_s_3" in "isatab/validate/test_core.py" is a good example. There are 2 different columns with warnings, but before this change you only saw 1 of those warnings, but those 2 warnings are repeated for each row. I'm not sure what would be the preference for how to handle this. I thought that maybe before issuing the warning there could be a check to see if the exact same warning was already issued and not do it again, but implementing this in a robust way did not seem trivial. For now, I simply left it so that the warnings/errors are repeated and changed the tests to match that where necessary.

coveralls commented 6 months ago

Coverage Status

coverage: 81.258% (+0.001%) from 81.257% when pulling 52c376f46062d9e05b6493f21830cfca6384a054 on ptth222:validate-config-changes into 7d5f19f9c7d2738849b2ac62e6d3c71603da8151 on ISA-tools:issue-511.

proccaserra commented 5 months ago

further discussions needed: considered to be a draft

main concern is the usability and confusion of adding a 'no config' parameter when a configuration is passed as well