Skip to content

Remove unused elementpath check in SHHS sleep staging task - #1290

Open
cpruijsen wants to merge 1 commit into
sunlabuiuc:masterfrom
cpruijsen:fix/issue-1282-63271745
Open

cpruijsen wants to merge 1 commit into
sunlabuiuc:masterfrom
cpruijsen:fix/issue-1282-63271745

Conversation

@cpruijsen

Copy link
Copy Markdown

Title: Fix SHHS sleep staging dependency check leaving XML parser undefined

Fixes #1282.

sleep_staging_shhs_fn in pyhealth/tasks/sleep_staging.py could not parse SHHS labels. Its dependency check passed a list to importlib.metadata.version(), which takes one distribution name, so it always raised TypeError. The bare except swallowed that, import xml.etree.ElementTree as ET never ran, and ET.fromstring raised UnboundLocalError.

The check guarded elementpath, which the function never uses and the package does not declare. This PR removes the check and imports ElementTree at module level.

Changes

  • pyhealth/tasks/sleep_staging.py: remove the dependency check and import xml.etree.ElementTree at the top of the module.
  • tests/core/test_sleep_staging_shhs.py: new tests. One checks that ET is bound at module level. One runs sleep_staging_shhs_fn end to end with a stubbed EDF read and an SHHS-shaped XML annotation file, and checks the labels and saved epochs.
  • docs/api/tasks/pyhealth.tasks.sleep_staging.rst: note that SHHS annotation parsing needs only the standard library.
  • examples/eeg/sleep_staging/sleep_staging_shhs_contrawr.py: remove the unused ContraWR import.

The issue offers two fixes: call version("elementpath"), or remove the check. I removed it, because a repaired check would either ask users to install a package the function does not need or add a new hard dependency. If you prefer to keep a check, I can restore it.

Both new tests fail on the current code and pass with this change.

The dependency check passed a list to importlib.metadata.version(),
which expects a single distribution name. It always raised TypeError,
was swallowed by the blanket except, and left ElementTree unbound, so
sleep_staging_shhs_fn could never parse SHHS labels. ElementTree is the
standard library and the only XML API the function uses; elementpath
was never referenced and is not a declared dependency. Import ElementTree
at module top and drop the check.

This branch has not been deployed

No deployments
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 this pull request may close these issues.

Bug: SHHS sleep staging fails dependency check and leaves XML parser undefined Copy title

1 participant