search

bids-standard / eye2bids

Convert eyetracking data to a BIDS compatible format (BEP20)
MIT License
5 stars 6 forks source link

[ENH] Changes for when RecordedEye = "Both" #40

Closed julia-pfarr closed 10 months ago

julia-pfarr commented 10 months ago

Changes according to https://github.com/bids-standard/eye2bids/issues/37.

Also: I changed functions for Average and Maximal Calibration Error so that the array containing error for left and right will be divided in arrays based on Calibration Counts. So if CalibrationCount = 1 the array looks like this: [[0.29], [0.35]] (=errors for 1 calibration of left and right) but when calibration count is 2 the array looks like this: [[[0.29], [0.35]], [[0.29], [0.35]]] to separate per calibration.

Remi-Gau commented 10 months ago

The diff seems too big and it seems the PR includes some of our old commits.

Try merging the upstream/master branch into your branch to help fix that.

julia-pfarr commented 10 months ago

The diff seems too big and it seems the PR includes some of our old commits.

Try merging the upstream/master branch into your branch to help fix that.

I did and now the diff is right. Don't know how to get rid of the old commits though, sorry

Remi-Gau commented 10 months ago

I did and now the diff is right. Nice

Don't know how to get rid of the old commits though, sorry. No worries, we will "squash and merge" the PR into a single commit, so we won't have duplicates in the history. It is technically possible to do this with some git foo, so those commits don't even appear in the PR but let's not bother with this for now.

Remi-Gau commented 10 months ago

does one of our test datasets has this usecase, if so we should have a test for it

Remi-Gau commented 10 months ago

does one of our test datasets has this usecase, if so we should have a test for it

for example here: https://github.com/bids-standard/eye2bids/blob/main/tests/test_edf2bids.py#L358

julia-pfarr commented 10 months ago

does one of our test datasets has this usecase, if so we should have a test for it

Not yet, but Martin sent me one of his files. I uploaded it to OSF ("2eyes")

Remi-Gau commented 10 months ago

OK let's add some tests because the errors that mypy is giving makes me think that this code may not run quite as we expect...

Remi-Gau commented 10 months ago

@julia-pfarr anything else you wanted to do here?

julia-pfarr commented 10 months ago

@julia-pfarr anything else you wanted to do here?

Nope, go ahead!