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

EDF.jl does not allow truncating header values to 0, even when it should #74

Open
ericphanson opened this issue Oct 17, 2023 · 0 comments · May be fixed by #75
Open

EDF.jl does not allow truncating header values to 0, even when it should #74

ericphanson opened this issue Oct 17, 2023 · 0 comments · May be fixed by #75
Assignees

Comments

@ericphanson
Copy link
Member

ericphanson commented Oct 17, 2023

Here I use OndaEDF to more easily construct the EDF.File object, but I believe the bug is in EDF.write.

Setup:

using OndaEDF, Onda, EDF
info = SamplesInfoV2(; sensor_type="x",
    channels=["x"],
    sample_unit="microvolt",
    sample_resolution_in_unit=1,
    sample_offset_in_unit=0,
    sample_type=Float64,
    sample_rate=1)

data = reshape([1e-9, 1], 1,:)
samples = Samples(data, info, false)
edf = onda_to_edf([samples])

The resulting EDF looks like:

julia> edf.header
EDF.FileHeader("0", EDF.PatientID(missing, missing, missing, missing), EDF.RecordingID(missing, missing, missing, missing), DateTime("1985-01-01T00:00:00"), true, 2, 1.0)

julia> edf.signals
1-element Vector{Union{EDF.AnnotationsSignal, EDF.Signal{Int16}}}:
 EDF.Signal{Int16}(EDF.SignalHeader("X X", "", "uV", 1.0f-9, 1.0f0, -32768.0f0, 32767.0f0, "", 1), Int16[-32768, 32767])

julia> edf.signals[1].header.physical_minimum
1.0f-9

When we try to write it:

julia> EDF.write( IOBuffer(), edf)
ERROR: failed to fit number into EDF's 8 ASCII character limit: 1.0e-9
Stacktrace:
 [1] error(s::String)
   @ Base ./error.jl:35
 [2] _edf_repr(x::Float32)
   @ EDF ~/EDF.jl/src/write.jl:37
 [3] edf_write(io::IOBuffer, value::Float32, byte_limit::Int64)
   @ EDF ~/EDF.jl/src/write.jl:49
 [4] write_header(io::IOBuffer, file::EDF.File{Int16, IOBuffer})
   @ EDF ~/EDF.jl/src/write.jl:95
 [5] write(io::IOBuffer, file::EDF.File{Int16, IOBuffer})
   @ EDF ~/EDF.jl/src/write.jl:180
 [6] top-level scope
   @ REPL[118]:1

The issue: EDF.jl does not want to round the physical_minimum down to zero, although that would be perfectly fine.

I think it is odd that the current _edf_repr code allows truncation to any value except 0, and if we have semantic concerns about truncating to 0, that should be checked in the EDF.File (or Signal) constructors on the appropriate fields, rather than applying to all numeric fields in _edf_repr.

@ericphanson ericphanson changed the title failed to fit number into EDF's 8 ASCII character limit: 1.0e-9 EDF.jl does not allow truncating header values to 0, even when it should Oct 17, 2023
@ericphanson ericphanson linked a pull request Oct 18, 2023 that will close this issue
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

Successfully merging a pull request may close this issue.

1 participant