Support signal annotations in NeoMatlabIO

[lkoelman]

Add support for reading basic annotations in NeoMatlabIO.

[pep8speaks]

Hello @lkoelman! Thanks for updating this PR. We checked the lines you've touched for PEP 8 issues, and found:

• In the file neo/io/neomatlabio.py:

Line 291:47: W504 line break after binary operator

Comment last updated at 2020-08-28 09:15:01 UTC

[samuelgarcia]

Hi @lkoelman.
Thank you for this.
Could you add some test about this in neo/test/iotest/test_neomatlabio.py ?
make a new method in TestNeoMatlabIO.

Samuel

[apdavison]

@lkoelman will you have time to add some tests for this? If you can provide this soon, we can include this in the upcoming 0.8.0 release

[lkoelman]

Hi there, my apologies for ignoring this. I'm running into my PhD submission deadline so not keen on spending time on anything else at the moment. Also not sure if my implementation is robust to more complex annotations. I could revisit this afterwards.

…

On Tue, Jul 23, 2019, 9:55 AM Andrew Davison ***@***.***> wrote: @lkoelman <https://github.com/lkoelman> will you have time to add some tests for this? If you can provide this soon, we can include this in the upcoming 0.8.0 release — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#633?email_source=notifications&email_token=AA2MARU4JHICBATWB7VU7QLQA3BQ3A5CNFSM4GQEJ3TKYY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOD2SNIMY#issuecomment-514118707>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AA2MARWM4BDXSHGLRBZRXT3QA3BQ3ANCNFSM4GQEJ3TA> .

[apdavison]

@lkoelman thanks for the quick reply. I'll postpone this to the next milestone.

[apdavison]

@lkoelman do you expect to have any time soon to add a test to this PR?

[lkoelman]

To be frank: not until after I finish my job search. Technical challenges are the new normal and they give me enough programming work for the moment.

…

On Sat, Apr 25, 2020, 8:02 PM Andrew Davison ***@***.***> wrote: @lkoelman <https://github.com/lkoelman> do you expect to have any time soon to add a test to this PR? — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#633 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AA2MARUWPRMP2QL45GCXRGLROMXURANCNFSM4GQEJ3TA> .

[samuelgarcia]

@mdenker : do you think you will have time to add some tests ?

[mdenker]

@samuelgarcia Yes, I'll have a look and will cook up some tests. It seems we need to fix a small conflict as well, since there was already a fix for annotations in a previous PR.

[samuelgarcia]

@mdenker @lkoelman
Hi. I have rebase this PR and added a very small test.
I think this is anough for now.

Could you have quick look ?

If it pass tests, I think thius is ready to merge.

[mdenker]

Hi Samuel, thanks for the additions. I am currently looking into this -- I still get an unit test assertion error on my side, investigating. Also, to cross-check, I'm comparing the fixes with an ancient branch https://github.com/INM-6/python-neo/tree/feature/matlab_annotations. Hope to get this sorted out by today.

[samuelgarcia]

Hi Michael.
Could you sen dyour error trace ?

[mdenker]

Still not sure, why I got errors, but I'm now convinced this was something strange on my side. Can confirm that this works and produces a workable matlab file, and it's compatible with the ancient branch.

What is not yet addressed is annotations which are a Quantity object. Values are correctly saved, but the unit is lost. I think this is a feature that should be handled by a separate PR. I opened #852 for this.