search

holgern / pyedflib

pyedflib is a python library to read/write EDF+/BDF+ files based on EDFlib.
http://pyedflib.readthedocs.org/
BSD 3-Clause "New" or "Revised" License
209 stars 121 forks source link

gender → sex #212

Closed DimitriPapadopoulos closed 12 months ago

DimitriPapadopoulos commented 1 year ago

Fixes #207 .

DimitriPapadopoulos commented 1 year ago

Ouch. Somehow I lost the gender branch. Will retry.

skjerns commented 1 year ago

Let me know when you are done then I'll review.

Also: Could you sqash the commits after finishing?

DimitriPapadopoulos commented 1 year ago

The new code should be backwards compatible enough. Not sure if I test the backwards compatibility stuff enough.

Can you have a look?

skjerns commented 1 year ago

lgtm

for testing it would be good to also check that the old functions still return the same values. Easiest would be to assert an assertion with getSex()==getGender() for all the replaced instances in the test suite.

DimitriPapadopoulos commented 1 year ago

About "Due to the edf specifications, only binary assignment is possible": I am not a specialist, but I understand most specialists usually describe biological sex as binary, based on what gametes one produces, while fully endorsing gender diversity. See for example https://doi.org/10.1002/bies.202200173. What could be more subjective is the use of biological sex as a variable of interest in situations where it is not relevant.

DimitriPapadopoulos commented 1 year ago

We do have a similar test in pyedflib/tests/test_edfreader.py:

        np.testing.assert_equal(f.getSex(), 'Male')
        np.testing.assert_equal(f.getGender(), 'Male')  # deprecated

I have just added tests to pyedflib/tests/test_edfwriter.py in test_sex_setting_correctly().

skjerns commented 12 months ago

perfect, I think it's ready to merge now :)