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

Newchecker #63

Closed Slowblitz closed 3 years ago

Slowblitz commented 3 years ago

Taking over #61 : So as we discuss earlier I'll continue this PR, there is still some stuff to be done as describe below :

Feel free to add task that I have maybe forgot =)

JuliaSprenger commented 3 years ago

Hi @Slowblitz I pushed some updates directly to your branch, don't be surprised when pushing the next time ;) However, tests are still failing :/

Slowblitz commented 3 years ago

Yes test are still failing =/ but thx for the update, so I have to pull now, so we don't get any conflict . Also I guess we have to change all dataset descriptions in our dataset to fit the test no ?

JuliaSprenger commented 3 years ago

I also adjusted the datasets. The failing tests are related to the old version of the generator in this branch and with the new version the tests are actually succeeding, see https://github.com/INT-NIT/AnDO/pull/62

Slowblitz commented 3 years ago

Ok so maybe it's time to delete the old one no ?

JuliaSprenger commented 3 years ago

Yes, we can do that. I hope this won't mess up me rebasing the new generator on this branch :crossed_fingers:

JuliaSprenger commented 3 years ago

@Slowblitz so should I or do you want to do this?

Slowblitz commented 3 years ago

You can do it =)

JuliaSprenger commented 3 years ago

Tadaaa

Slowblitz commented 3 years ago

\o/ yeaaaa good job =)

JuliaSprenger commented 3 years ago

Regarding the last open checkbox for this PR, the error handling, I suggest to leave it as it is right now (the is_valid function returning a bool indicator and a list of error messages as this output is well defined and can be used by different interfaces (python scripts, webchecker,...)

SylvainTakerkart commented 3 years ago

OK, the tests are failing coz there's a problem with a relative import... I need to fix this!

SylvainTakerkart commented 3 years ago

@JuliaSprenger OK, tests pass! I let you merge if you want to take a look (or merge directly)...