fix writing of decimal sample frequency #159

skjerns · 2022-01-18T21:15:48Z

In this PR I'm fixing some mistakes that have been made before, and include some clarifications w.r.t to sampling_frequency and sample_rate

Summary:

sample_frequency is now the main term being used and denoted the samples in Hz (samples per second)
we hide/discourage the use of sample_rate. To prevent confusion, for now we pretend sample_frequency and sample_rate are the same term towards the user.
the use of record_duration was entirely wrong. Fixed that now.

Terms:
smp_per_record: tells us how many samples there are in each record.
record_duration: denotes the time window of one record. If all sampling frequencies are ints, this can simply be 1.
sample_frequency: is calculated by dividing smp_per_record/record_duration. If we write a file, we have to do the reverse, and find an optimal record_duration such that can actually represent the wished sample_frequency.

Problem:
Sampling frequency (samples per seconds in Hz) is not explicitly denoted in EDF. It is calculated by smp_per_record (samples, int) / record_duration (seconds, float). Previously it was just assumed that sample_rate is equivalent to sample_frequency, which is not the case, as sample_rate is taken directly from smp_per_record. However, when the record_duration is unequal to 1, this is not the same as the sampling frequency.
Similarly, if we want to save a file with decimal sampling_frequency, e.g. 0.5 Hz, we need to make the equation 0.5=smp_per_record/record_duration work such that smp_per_record is an int. That means we need to look for a record_duration that matches the smp_per_record for all channels in the EDF. I posted a simplified solution that should work in most cases (just try out record_duration for 1s to 60s in 1s steps), however, we can think of some more mathematical solution as well later.

Proposition
I think nobody cares how many samples are stored within a record_duration. These are technical details. People want to set a sampling frequency. They do not care how this is realized and stored within the file. Therefore I opt to remove any setting of record_duration and ``

closes #148 #111

This is a rather large commit, and I would appreciate if someone would take a close look at it @holgern

Build Python 3.10 wheels for aarch64

Run tests on Python 3.10

codecov · 2022-01-18T21:16:01Z

Codecov Report

Patch coverage: 76.92% and project coverage change: +5.00 🎉

Comparison is base (b92d2f9) 76.08% compared to head (ff82c93) 81.08%.

Additional details and impacted files

@@            Coverage Diff             @@
##           master     #159      +/-   ##
==========================================
+ Coverage   76.08%   81.08%   +5.00%     
==========================================
  Files           4        4              
  Lines         828      809      -19     
  Branches      176      170       -6     
==========================================
+ Hits          630      656      +26     
+ Misses        156      112      -44     
+ Partials       42       41       -1

Flag	Coverage Δ
unittests	`81.08% <76.92%> (+5.00%)`	⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files	Coverage Δ
pyedflib/highlevel.py	`72.72% <0.00%> (-0.90%)`	⬇️
pyedflib/edfwriter.py	`86.50% <77.08%> (+4.49%)`	⬆️
pyedflib/edfreader.py	`80.95% <100.00%> (+13.13%)`	⬆️

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

☔ View full report at Codecov.
📢 Do you have feedback about the report comment? Let us know in this issue.

skjerns · 2022-01-18T21:17:00Z

pyedflib/edfreader.py

+        nsigs = int(header['n_signals'])
+        label = [f.read(16).decode() for i in range(nsigs)]
+        transducer = [f.read(80).decode().strip() for i in range(nsigs)]
+        dimension = [f.read(8).decode().strip() for i in range(nsigs)]
+        pmin = [f.read(8).decode() for i in range(nsigs)]
+        pmax = [f.read(8).decode() for i in range(nsigs)]
+        dmin = [f.read(8).decode() for i in range(nsigs)]
+        dmax = [f.read(8).decode() for i in range(nsigs)]
+        prefilter = [f.read(80).decode().strip() for i in range(nsigs)]
+        n_samples = [f.read(8).decode() for i in range(nsigs)]
+        reserved = [f.read(32).decode() for i in range(nsigs)]
+    _ = zip(label, transducer, dimension, pmin, pmax, dmin, dmax, prefilter, n_samples, reserved)
+    values = locals().copy()
+    fields = ['label', 'transducer', 'dimension', 'pmin', 'pmax', 'dmin', 'dmax', 'prefilter', 'n_samples', 'reserved']
+    sheaders = [{field:values[field][i] for field in fields} for i in range(nsigs)]
+    print('\n##### Signal Headers')
+    print(json.dumps(sheaders, indent=2))


unrelated, just ignore, just for debugging

skjerns · 2022-01-18T21:17:22Z

pyedflib/edfwriter.py

-from ._extensions._pyedflib import set_startdatetime, set_starttime_subsecond, set_samplefrequency, set_physical_minimum, set_label, set_physical_dimension
+from ._extensions._pyedflib import set_startdatetime, set_starttime_subsecond, set_samples_per_record, set_physical_minimum, set_label, set_physical_dimension


renamed to accurately reflect what the function actually does.

skjerns · 2022-01-18T21:18:56Z

pyedflib/edfwriter.py

+        record_duration : integer
+            Sets the datarecord duration in units of seconds


This was wrong. We insert the record_duration in seconds, not 10mS (this is how it is saved later inside the edf, but not how we indicate it here)

skjerns · 2022-01-18T21:19:37Z

pyedflib/highlevel.py

-    block_size : int
-        set the block size for writing. Should be divisor of signal length
-        in seconds. Higher values mean faster writing speed, but if it
-        is not a divisor of the signal duration, it will append zeros.
-        Can be any value between 1=><=60, -1 will auto-infer the fastest value.


the use of block_size was entirely wrong here, my bad. needs to be removed.

skjerns · 2022-01-18T21:20:04Z

pyedflib/highlevel.py

    # get annotations, in format [[timepoint, duration, description], [...]]
    annotations = header.get('annotations', [])

    with pyedflib.EdfWriter(edf_file, n_channels=n_channels, file_type=file_type) as f:
-        f.setDatarecordDuration(int(100000 * block_size))


completely misunderstood the use of record_duration

skjerns · 2022-01-18T21:20:55Z

pyedflib/tests/test_edfreader.py

+
+    def tearDown(self):
+        # small hack to close handles in case of tests throwing an exception
+        for obj in gc.get_objects():
+            if isinstance(obj, (EdfWriter, EdfReader)):
+                obj.close()
+                del obj
+


if a test fails it will leave the file open. if the same file is accessed in a later test, that will fail again, just due to the file still being open.

added this to close all files that were not closed because a test failed.

skjerns · 2022-01-18T21:21:25Z

pyedflib/tests/test_edfreader.py

-        sample_frequencies = [1000, 800, 500, 975, 999]
+        sample_frequencies = [2000, 1600, 1000, 1950, 1998]



these were wrong before. Now they are correct and finally are also what EDFBrowser would display.

skjerns · 2022-01-18T21:21:30Z

pyedflib/tests/test_edfreader.py

-        sample_frequencies = [1000, 800, 500, 975, 999]
+        sample_frequencies = [500, 400, 250, 487.5, 499.5]


these were wrong before. Now they are correct and finally are also what EDFBrowser would display.

skjerns · 2022-01-18T21:22:00Z

pyedflib/tests/test_highlevel.py

@@ -156,11 +166,14 @@ def test_fortran_write(self):


    def test_read_write_decimal_sample_frequencies(self):
-        signals = np.random.randint(-2048, 2048, [3, 256*60])
+        # first test with digital signals
+        signals = np.random.randint(-2048, 2048, [3, 256*60+8])


+8 as the file is padded by zeros else.

Add tests for ChannelDoesNotExist Exception

…mplerate2

Cleanup CIs

Fix importlib

change win2016 to win2019 as its deprecated

remove deprecated "mode" parameter

…ue148_samplerate2

Fix writing of annotations in very long records

…ue148_samplerate2

Remove commented code and merged from develop

skjerns · 2023-03-02T14:42:54Z

I changed using @master to @v3 and @v4 in GitHub actions as suggested generally in actions/upload-artifact#41 (not specifically for Python setup in GitHub actions, but is probably generally better to use a released version instead of the master/main branch)

Also Python 3.6 is removed from testing as its no longer available.

… to int

skjerns · 2023-03-03T10:16:40Z

Went once through all the changes, added some more comments. I think all changes are okay and it's long overdue to merge this PR

Thanks for @gcathelain for reviewing!

I'll merge it soon.

skjerns and others added 11 commits January 4, 2022 10:49

Merge pull request #154 from cbrnr/python310

ff39881

Build Python 3.10 wheels for aarch64

Run tests on Python 3.10

596ced5

3.10 on CircleCI

ba47528

AppVeyor Python 3.10

9cc6df5

replace lines in accordance with edflib 121

5fdd3e5

Merge pull request #158 from holgern/fix_bug

4d99bb9

Replace nose with pytest

e8c93c3

Merge pull request #155 from cbrnr/test-py310

8982f26

Run tests on Python 3.10

fix writing of decimal sample frequency

2fec68c

add a warning

78ef9e7

change duration to record_duration

2ee97a6

skjerns commented Jan 18, 2022

View reviewed changes

skjerns added 5 commits January 18, 2022 23:21

Update test_edfwriter.py

b1ca265

Update test_edfwriter.py

508839f

Merge pull request #160 from holgern/test_exceptions

f8c57e9

Add tests for ChannelDoesNotExist Exception

Merge branch 'master' of github.com:holgern/pyedflib into issue148_sa…

920a5f8

…mplerate2

remove CircleCI

02d4350

skjerns mentioned this pull request Jan 19, 2022

Decimal sample rate compatibility #118

Closed

holgern and others added 3 commits January 19, 2022 13:59

Merge pull request #162 from holgern/cleanup_ci2

d2b6471

Cleanup CIs

remove tox & adveyor

ca0b738

add windows-2016

b1af6d8

holgern and others added 8 commits February 21, 2022 15:06

Release 0.1.28

62515d5

Fix importlib

Update README.rst

1aab741

remove deprecated "mode" parameter

dbed52a

change win2016 to win2019 as its deprecated

af9614f

Merge pull request #173 from holgern/change_ci

a9225fa

change win2016 to win2019 as its deprecated

Merge branch 'master' of https://github.com/holgern/pyedflib into fix171

d563420

Merge pull request #172 from holgern/fix171

a616138

remove deprecated "mode" parameter

Merge branch 'master' of https://github.com/holgern/pyedflib into iss…

fae32d6

…ue148_samplerate2

skjerns mentioned this pull request May 6, 2022

Cannot write annotations for long recordings #175

Closed

skjerns added 5 commits May 10, 2022 16:51

fix writing of annotations in very long records

31f3af5

fix typo

2d4585b

Merge pull request #176 from holgern/fix_long_duration

4aa7fe1

Fix writing of annotations in very long records

Merge branch 'master' of https://github.com/holgern/pyedflib into iss…

27fdfc4

…ue148_samplerate2

remove debug parser from coverage

9e0d38e

skjerns mentioned this pull request Jun 25, 2022

"sample_frequency" is a misleading name for the number of values in a signal #148

Closed

skjerns force-pushed the master branch from c7ca168 to af73377 Compare September 27, 2022 07:34

skjerns mentioned this pull request Jan 16, 2023

Wrong signal sample rate if duration of a data record in EDF header is not 1 second #179

Closed

skjerns requested a review from holgern January 24, 2023 15:42

skjerns mentioned this pull request Feb 28, 2023

How to remove the limit on the number of annotations? #187

Open

gcathelain added 2 commits March 2, 2023 08:51

Merge branch 'master' into issue148_samplerate2

00950ca

removing commented code

6eca779

gcathelain mentioned this pull request Mar 2, 2023

Remove commented code and merged from develop #188

Merged

Merge pull request #188 from gcathelain/issue148_samplerate2

576a7d8

Remove commented code and merged from develop

skjerns added 2 commits March 2, 2023 15:43

change from @master to explicit versioning as recommended

ebf2992

Add comments, remove whitespace, rename vars, add explicit conversion…

b20f92a

… to int

change copyright and version

ff82c93

skjerns merged commit f094d69 into master Mar 3, 2023

skjerns mentioned this pull request Apr 19, 2023

Write EDF with sample rate < 1 #111

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

fix writing of decimal sample frequency #159

fix writing of decimal sample frequency #159

skjerns commented Jan 18, 2022 •

edited

Loading

codecov bot commented Jan 18, 2022 •

edited

Loading

skjerns Jan 18, 2022

skjerns Jan 18, 2022

skjerns Jan 18, 2022

skjerns Jan 18, 2022

skjerns Jan 18, 2022

skjerns Jan 18, 2022

skjerns Jan 18, 2022

skjerns Jan 18, 2022

skjerns Jan 18, 2022

skjerns commented Mar 2, 2023 •

edited

Loading

skjerns commented Mar 3, 2023 •

edited

Loading

		from ._extensions._pyedflib import set_startdatetime, set_starttime_subsecond, set_samplefrequency, set_physical_minimum, set_label, set_physical_dimension
		from ._extensions._pyedflib import set_startdatetime, set_starttime_subsecond, set_samples_per_record, set_physical_minimum, set_label, set_physical_dimension

		record_duration : integer
		Sets the datarecord duration in units of seconds

		sample_frequencies = [1000, 800, 500, 975, 999]
		sample_frequencies = [2000, 1600, 1000, 1950, 1998]

		sample_frequencies = [1000, 800, 500, 975, 999]
		sample_frequencies = [500, 400, 250, 487.5, 499.5]

fix writing of decimal sample frequency #159

fix writing of decimal sample frequency #159

Conversation

skjerns commented Jan 18, 2022 • edited Loading

codecov bot commented Jan 18, 2022 • edited Loading

Codecov Report

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

skjerns commented Mar 2, 2023 • edited Loading

skjerns commented Mar 3, 2023 • edited Loading

skjerns commented Jan 18, 2022 •

edited

Loading

codecov bot commented Jan 18, 2022 •

edited

Loading

skjerns commented Mar 2, 2023 •

edited

Loading

skjerns commented Mar 3, 2023 •

edited

Loading