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.

Sign up for GitHub

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

Write onsets and durations with more decimal digits #72

Open
wants to merge 5 commits into
base: eph/mne-tests
Choose a base branch
from
Open

Write onsets and durations with more decimal digits #72

Show file tree
Hide file tree
Changes from 4 commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
7dc31ca
write onsets and durations with more digits
ericphanson Oct 16, 2023
9b08549
Merge remote-tracking branch 'origin/eph/mne-tests' into eph/more-digits
ericphanson Oct 16, 2023
c5cd5c1
Merge remote-tracking branch 'origin/eph/mne-tests' into eph/more-digits
ericphanson Oct 16, 2023
644dc04
rm rounding
ericphanson Oct 16, 2023
d2bd69c
Update test/runtests.jl
ericphanson Oct 16, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
17 changes: 0 additions & 17 deletions src/types.jl
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -89,12 +89,6 @@ const ANNOTATIONS_SIGNAL_LABEL = ["EDF Annotations", "BDF Annotations"]

A type representing a time-stamped annotations list (TAL).

Note that this type's constructor may attempt to round given `onset_in_seconds` and
`duration_in_seconds` arguments to their nearest representable values in accordance
with the EDF+ specification, which a) represents these values as ASCII, b) constrains
these values to an 8 character limit, and c) does not allow the use of scientific
notation for these fields.

See EDF+ specification for details.

# Fields
Expand All @@ -107,17 +101,6 @@ struct TimestampedAnnotationList
onset_in_seconds::Float64
duration_in_seconds::Union{Float64,Nothing}
annotations::Vector{String}
function TimestampedAnnotationList(onset_in_seconds, duration_in_seconds, annotations)
onset_in_seconds = _nearest_representable_edf_time_value(onset_in_seconds)
duration_in_seconds = _nearest_representable_edf_time_value(duration_in_seconds)
return new(onset_in_seconds, duration_in_seconds, annotations)
end
end

_nearest_representable_edf_time_value(::Nothing) = nothing

function _nearest_representable_edf_time_value(x)
return round(x; digits=(8 - (ndigits(floor(Int, x)) + signbit(x) + isinteger(x))))
end

function Base.:(==)(a::TimestampedAnnotationList, b::TimestampedAnnotationList)
Expand Down
4 changes: 2 additions & 2 deletions src/write.jl
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -138,10 +138,10 @@ function write_tal(io::IO, tal::TimestampedAnnotationList)
if !signbit(tal.onset_in_seconds) # otherwise, the `-` will already be in number string
bytes_written += Base.write(io, '+')
end
bytes_written += Base.write(io, _edf_repr(tal.onset_in_seconds))
bytes_written += Base.write(io, @sprintf("%f", tal.onset_in_seconds))
if tal.duration_in_seconds !== nothing
bytes_written += Base.write(io, 0x15)
bytes_written += Base.write(io, _edf_repr(tal.duration_in_seconds))
bytes_written += Base.write(io, @sprintf("%f", tal.duration_in_seconds))
end
if isempty(tal.annotations)
bytes_written += Base.write(io, 0x14)
Expand Down
28 changes: 19 additions & 9 deletions test/runtests.jl
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -81,7 +81,7 @@ const DATADIR = joinpath(@__DIR__, "data")
[TimestampedAnnotationList(4.0, nothing, String[""]), TimestampedAnnotationList(2.5, 2.5, ["type A"])],
[TimestampedAnnotationList(5.0, nothing, String[""])]]
@test all(signal.records .== expected)
@test AnnotationsSignal(signal.records).samples_per_record == 16
@test AnnotationsSignal(signal.records).samples_per_record == 20
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

more bytes per TAL (since more digits) means a higher number here, IIUC

end
end

Expand All @@ -108,6 +108,23 @@ const DATADIR = joinpath(@__DIR__, "data")
@test pyconvert(Float64, ann["duration"]) == 0.256
@test pyconvert(String, ann["description"]) == "type A"

# Modify it to have a start time with high precision
edf_high = @set edf.signals[end].records[1][2].onset_in_seconds = 1e-7
@test edf_high.signals[end].records[1][2].onset_in_seconds == 1e-7
py = mne_read(edf_high)
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this errors on main (or rather, on #71) because we hit the failed to fit number into EDF's 8 ASCII character limit path in _edf_repr here when going through EDF.write. On this branch it does not error because we just write it out with all digits, even if MNE will round them down later.

ann = py.annotations[0]
# MNE drops the precision, but doesn't error
@test pyconvert(Float64, ann["onset"]) == 0
@test pyconvert(Float64, ann["duration"]) == 0

edf_high = @set edf.signals[end].records[1][2].onset_in_seconds = 0.1055043123123120
@test edf_high.signals[end].records[1][2].onset_in_seconds == 0.1055043123123120
py = mne_read(edf_high)
ann = py.annotations[0]
# MNE doesn't read it in with the last digit, but it also doesn't error:
@test pyconvert(Float64, ann["onset"]) == 0.105504
@test pyconvert(Float64, ann["duration"]) == 0

# test that EDF.write(::IO, ::EDF.File) errors if file is
# discontiguous w/o an AnnotationsSignal present
bad_file = EDF.File(IOBuffer(),
Expand All @@ -133,13 +150,6 @@ const DATADIR = joinpath(@__DIR__, "data")
@test eof(io)
end

@test EDF._edf_repr(EDF._nearest_representable_edf_time_value(-0.0023405432)) == "-0.00234"
@test EDF._edf_repr(EDF._nearest_representable_edf_time_value(0.0023405432)) == "0.002340"
@test EDF._edf_repr(EDF._nearest_representable_edf_time_value(1.002343)) == "1.002343"
@test EDF._edf_repr(EDF._nearest_representable_edf_time_value(1011.05432)) == "1011.054"
@test EDF._edf_repr(EDF._nearest_representable_edf_time_value(-1011.05432)) == "-1011.05"
@test EDF._edf_repr(EDF._nearest_representable_edf_time_value(-1013441.5)) == "-1013442"
@test EDF._edf_repr(EDF._nearest_representable_edf_time_value(-1013441.3)) == "-1013441"
@test EDF._edf_repr(34577777) == "34577777"
@test EDF._edf_repr(0.0345) == "0.034500"
@test EDF._edf_repr(-0.02) == "-0.02000"
Expand Down Expand Up @@ -267,7 +277,7 @@ const DATADIR = joinpath(@__DIR__, "data")
edf = EDF.read(joinpath(DATADIR, "test_float_extrema.edf"))
@test edf.signals[1].header.digital_minimum ≈ -32767.0f0
edf = @set edf.signals[1].header.digital_minimum = -32767 * 2

@test edf.signals[1].header.digital_minimum == -32767 * 2 # double-check it got set
ericphanson marked this conversation as resolved.
Show resolved Hide resolved
py = mne_read(edf)
@test isempty(py.annotations)
end
Expand Down