isa-api
isa-api copied to clipboard
Changed missing required properties to error
If a property is "required" it should be an error, no?
I also changed the investigation config file so that Study Person Mid Initial and Investigation Person Mid Initial aren't required. If this shouldn't be, feel free to put it back. It just seems odd to me to require a middle initial.
It looks like some of the tests are specifically looking for the messages to be warnings. If it is agreed that these should be errors I think those tests have to change.
@ptth222 +1 that makes sense.
Adding my last commit message here:
There were quite a few changes because warnings became errors in a lot of tests.
isa_tab/validate/test_core.py test_b_ii_s_3: The old warnings were:
[{'message': 'Protocol declared but not used',
'supplemental': "protocols declared in the file s_BII-S-3.txt are not used in any assay file: {'sequence analysis - standard procedure 7', 'reverse transcription - standard procedure 5'}",
'code': 1019},
{'message': 'DOI is not valid format',
'supplemental': 'Found 10.1371/journal.pone.0003042 in DOI field',
'code': 3002},
{'message': 'DOI is not valid format',
'supplemental': 'Found 10.1111/j.1462-2920.2008.01745.x in DOI field',
'code': 3002},
{'message': 'A required property is missing',
'supplemental': 'A property value in Investigation Title of investigation file at column 1 is required',
'code': 4003},
{'message': 'A required property is missing',
'supplemental': 'A property value in Investigation Description of investigation file at column 1 is required',
'code': 4003},
{'message': 'A required property is missing',
'supplemental': 'A property value in Study Person Mid Initials of investigation file at column 2 is required',
'code': 4003},
{'message': 'A required property is missing',
'supplemental': 'A property value in Study Person Mid Initials of investigation file at column 3 is required',
'code': 4003},
{'message': 'A required property is missing',
'supplemental': 'A property value in Study Person Mid Initials of investigation file at column 4 is required',
'code': 4003},
{'message': 'A required property is missing',
'supplemental': 'A property value in Study Person Mid Initials of investigation file at column 5 is required',
'code': 4003},
{'message': 'A required property is missing',
'supplemental': 'A property value in Study Person Mid Initials of investigation file at column 6 is required',
'code': 4003},
{'message': 'A required property is missing',
'supplemental': 'A property value in Study Person Mid Initials of investigation file at column 7 is required',
'code': 4003},
{'message': 'A value does not correspond to the correct data type',
'supplemental': "Invalid value 'WGS' for type 'list' of the field 'Parameter Value[library strategy]'",
'code': 4011}]
The required property warnings became errors. The other tests in this same file that were changed all had similar circumstances.
test_clients/test_mw2isa.py test_conversion_ms: Required property warnings became errors, so I added their code, '4003', to the test.
validators/test_validate_test_data test_validate_testdata_sra_chromatin_mod_seq_isatab: The following warnings became errors, so the test for 0 errors was broken:
[{'message': 'A required property is missing',
'supplemental': 'A property value in Study Publication DOI of investigation file at column 1 is required',
'code': 4003},
{'message': 'A required property is missing',
'supplemental': 'A property value in Study Publication Author List of investigation file at column 1 is required',
'code': 4003},
{'message': 'A required property is missing',
'supplemental': 'A property value in Study Publication Title of investigation file at column 1 is required',
'code': 4003}]
I changed the test to allow for more errors, but that might not be the best solution. All of the changed tests in this file had the same circumstances.
convert/test_mzml2isa.py: The following warnings becamse errors:
[{'message': 'A required property is missing',
'supplemental': 'A property value in Investigation Title of investigation file at column 1 is required',
'code': 4003},
{'message': 'A required property is missing',
'supplemental': 'A property value in Investigation Description of investigation file at column 1 is required',
'code': 4003},
{'message': 'A required property is missing',
'supplemental': 'A property value in Study Title of investigation file at column 1 is required',
'code': 4003},
{'message': 'A required property is missing',
'supplemental': 'A property value in Study Description of investigation file at column 1 is required',
'code': 4003}]
This might suggest that there may be a problem with the converter if these values aren't being added.
coverage: 81.255% (-0.002%) from 81.257% when pulling 1a8dab1722e63ba96ec7544a745e030281a76c40 on ptth222:inv-config-change into 7d5f19f9c7d2738849b2ac62e6d3c71603da8151 on ISA-tools:issue-511.
The lines from the new commit need to be accepted. The tests had to change since this code changed a warning to an error.
Turned out neither of the lines worked. The DOI fix wasn't merged in yet I think. That fix reduced the warnings by 2, so I fixed the tests appropriately.
closing but changes integrated via another PR . Thank you @ptth222