search

INT-NIT / BEP032tools

Software tools supporting the BIDS Extension Proposal (BEP) dedicated to adding support for electrophysiological data recorded in animal models (BEP032)
MIT License
3 stars 8 forks source link

General problem with validation #9

Closed SylvainTakerkart closed 4 years ago

SylvainTakerkart commented 4 years ago

Hi, I've just detected a bug which, from what I understand, is due to the algorithm used to validate a dataset...

Here is the problem:

python AnDOChecker.py tests/ds002/data/Landing/ tests/ds002/data/Landing/: Is validated by AnDOChecker (this one is normal, it's what we expect)

python AnDOChecker.py tests/ds002/data/ tests/ds002/data/: Is validated by AnDOChecker (this one is NOT what we expect)

python AnDOChecker.py tests/ds002/ tests/ds002/: Is validated by AnDOChecker (this one is NOT what we expect)

This is caused by the algorithm used to validate... The line return any(flatten(conditions)) (line 83 of AnDO_engine.py) is simply false because it means "as soon as there is ONE name which is a correct subject name in all the list of names passed as argument, then it's validated by the function is_subject"

Same for lines 102 and 120 in the is_source and is_session functions!

This has to be totally re-thought!

I will add some datasets (which should not be validated) so this can be tested!

Slowblitz commented 4 years ago

Nice, I think we have to add the "exp-" "sess-" and "sub" rules prefix first as we discussed earlier, this can be helpful to check if the first folder is correct (i.e. "exp-landing") making it easier to check if the folder given in arg is at the right level.

SylvainTakerkart commented 4 years ago

I don't think this can be a full solution, I mean you cannot only have a rule "if name starts by exp-, we are at the experiment level" and rely on it...

so I think you should not add these prefixes, you should find a solution without the prefixes (by checking level 1, level 2, level 3 of the hierarchy, and respectively apply is_subject, is_session, is_source); we will add the prefixes later...

SylvainTakerkart commented 4 years ago

PR #11 fixed most of this, and PR #12 added some minor formatting adjustments. We're good to go now!