Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

wrong_parse_of_a_tal #9

Open
andrbru opened this issue Aug 13, 2022 · 9 comments
Open

wrong_parse_of_a_tal #9

andrbru opened this issue Aug 13, 2022 · 9 comments

Comments

@andrbru
Copy link

andrbru commented Aug 13, 2022

I might have found a file where the first TAL is read incorrectly.

Besides the file, I enclose a photo of comparsion of the annotations as displayed in Polyman (right) and in program I am implementing a EDF reading for (left) where I just used the new version of your library. It seems as if time and duration was read as a part of description.

Just one detail: in your namespace, a TAL means an annotation, i. e. message with timing? In EDF specs, it means a Time stamped Annotation List , i. e. list of messages sharing sa
files.zip
me timing.

@andrbru
Copy link
Author

andrbru commented Aug 13, 2022

More files where I believe the same is happening, in case you could have use of them for testing.
files2.zip

@LiorBanai
Copy link
Owner

I'll check :)

@LiorBanai
Copy link
Owner

I was confused with the specifications actually.

I just parsed each annotation from the files.
Do you mean I need to change the names?

LiorBanai added a commit that referenced this issue Aug 14, 2022
@LiorBanai
Copy link
Owner

hi @andrbru . I pushed a fix for this. should be OK now.

@andrbru
Copy link
Author

andrbru commented Aug 15, 2022

Good! I'll try the fixed version.

If I understand it correctly than your naming convention is other than the one used in EDF specs. But since some people are already using your library, it might not be good to change the names...

@LiorBanai
Copy link
Owner

I think it is better to fix the naming to match the spec for future use.

@andrbru
Copy link
Author

andrbru commented Aug 15, 2022

Besides form that, I noticed that your library loads a huge number of EDF Annotations signals from a file which Polyman displays as having only one. But I believe that's because Polyman always displays only one EDF Annotations signal regardless of how many there accualy are in file bytes.

Also, have I noticed correctly that if there are any EDF Annotations signal(s), your library adds one EDF Annotations signal into the array of standard signals? Why that?

@LiorBanai
Copy link
Owner

I think for backward compatibility the annotation are created as "standard" signals.

There are many improvements that can be done to the library. I just started working on it (based on other people work) not long ago..

@andrbru
Copy link
Author

andrbru commented Aug 15, 2022

It does't cause any trouble. I simply made an IF sorting that channel out so there won't be two of annotations signals.

I understand, there is allways sometrnig to improve. It's still the best EDF library for C# I have found, since it is the only one implementing annotations from EDF+. I hope the errors that I report here will help you improve it.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants