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

nwb to BIDS converter integration #73

Closed Saksham20 closed 3 years ago

Saksham20 commented 3 years ago
coveralls commented 3 years ago

Pull Request Test Coverage Report for Build 708976893

Changes Missing Coverage Covered Lines Changed/Added Lines %
ando/tools/generator/tests/test_nwb2bidsgenerator.py 0 20 0.0%
ando/tools/generator/nwb2bidsgenerator.py 0 114 0.0%
<!-- Total: 0 134 0.0% -->
Files with Coverage Reduction New Missed Lines %
ando/tests/test_AnDOChecker.py 2 98.15%
ando/AnDOChecker.py 12 71.79%
<!-- Total: 14 -->
Totals Coverage Status
Change from base Build 679700768: -0.5%
Covered Lines: 195
Relevant Lines: 620
💛 - Coveralls
JuliaSprenger commented 3 years ago

Hi Saksham, currently the checker is only imported but not used for validation after the AnDO structure is generated. Do you plan to add this feature or trust on the nwb2bidsgenerator here completely and leave it up to the user to do a validation later on?

pep8speaks commented 3 years ago

Hello @Saksham20! Thanks for updating this PR.

Line 26:44: E126 continuation line over-indented for hanging indent Line 26:101: E501 line too long (118 > 100 characters) Line 37:101: E501 line too long (108 > 100 characters) Line 45:65: E226 missing whitespace around arithmetic operator Line 57:54: E226 missing whitespace around arithmetic operator Line 57:75: E226 missing whitespace around arithmetic operator Line 59:65: E226 missing whitespace around arithmetic operator Line 59:101: E501 line too long (114 > 100 characters) Line 63:63: E226 missing whitespace around arithmetic operator Line 63:101: E501 line too long (110 > 100 characters) Line 68:66: E226 missing whitespace around arithmetic operator Line 68:101: E501 line too long (115 > 100 characters) Line 70:64: E226 missing whitespace around arithmetic operator Line 70:101: E501 line too long (111 > 100 characters) Line 75:65: E226 missing whitespace around arithmetic operator Line 75:101: E501 line too long (111 > 100 characters) Line 77:101: E501 line too long (101 > 100 characters) Line 168:51: E226 missing whitespace around arithmetic operator Line 168:61: E226 missing whitespace around arithmetic operator Line 184:45: E127 continuation line over-indented for visual indent Line 198:39: E226 missing whitespace around arithmetic operator Line 198:50: E226 missing whitespace around arithmetic operator Line 198:61: E226 missing whitespace around arithmetic operator Line 235:46: E226 missing whitespace around arithmetic operator

Line 9:1: E302 expected 2 blank lines, found 1 Line 12:24: E226 missing whitespace around arithmetic operator Line 38:32: E225 missing whitespace around operator Line 45:36: E225 missing whitespace around operator Line 45:36: E712 comparison to False should be 'if cond is False:' or 'if not cond:' Line 46:69: E231 missing whitespace after ',' Line 51:101: E501 line too long (104 > 100 characters)

Comment last updated at 2021-04-06 06:40:50 UTC
Saksham20 commented 3 years ago

@JuliaSprenger the tests seem to pass now, thanks for working on the CI file from your fork!

I have also made some changes to the conversion code:

I have made a BidsConverterclass which is then inhereted by the NwbToBIDS class which contains all the code. The base class has some methods which I think could be used by future converter from other formats to BIDS. We can refine the base class once more formats start coming in. Let me know if you think some more methods could be added to the base BidsConverter.

@Slowblitz @JuliaSprenger There is one problem though with the validator (is_valid()), it is returning True if an .nwb file is absent from the file structure. It only return the desired error when the file name is change to something random like 'Naming rule not respected for this file : asdf.nwb'.

SylvainTakerkart commented 3 years ago

@Slowblitz @JuliaSprenger There is one problem though with the validator (is_valid()), it is returning True if an .nwb file is absent from the file structure. It only return the desired error when the file name is change to something random like 'Naming rule not respected for this file : asdf.nwb'.

Thanks @Saksham20 for spotting this! Can you open an issue to describe it ? Cheers, Sylvain

JuliaSprenger commented 3 years ago

@Slowblitz did this already, see https://github.com/INT-NIT/AnDO/issues/79

JuliaSprenger commented 3 years ago

Hi @Saksham20! I fixed the tests for this PR in https://github.com/INT-NIT/AnDO/pull/77 and merged the nwb generator into master now. If you still have more comments / change requests feel free to comment here or add another issue / pr. Thanks for the contribution!