Skip to content

Fix CedRawIO with sonpy >= 1.9.12 and re-enable the CED tests - #1891

Open
AxelNoun wants to merge 5 commits into
NeuralEnsemble:masterfrom
AxelNoun:fix/cedrawio-sonpy-namespace
Open

Fix CedRawIO with sonpy >= 1.9.12 and re-enable the CED tests#1891
AxelNoun wants to merge 5 commits into
NeuralEnsemble:masterfrom
AxelNoun:fix/cedrawio-sonpy-namespace

Conversation

@AxelNoun

Copy link
Copy Markdown

Closes #1890.

sonpy 1.9.12 reorganised its package layout and dropped the lib namespace that
cedrawio.py relies on, so every sonpy.lib.* access raises
AttributeError: module 'sonpy' has no attribute 'lib'.

While preparing the fix I found that the CED tests could not have caught this, for two
independent reasons — so this PR fixes the reader and restores the test coverage that would
have flagged it.

1. neo/rawio/cedrawio.py — resolve the namespace

A cached _get_sonpy_namespace() helper probes three candidates in order:

candidate covers
sonpy.lib <= 1.9.5, the old per-platform dispatch
sonpy >= 1.9.12 on Windows and macOS (from .sonpy import *)
sonpy.sonpy >= 1.9.12 on Linux, whose wheel ships an empty __init__.py

The third is not redundant: the cp314-manylinux_2_39_x86_64 wheel has a 0-byte
__init__.py, so neither sonpy.lib nor sonpy.SonFile resolves there. If none of the
three exposes SonFile, an ImportError naming all three is raised, rather than letting an
AttributeError surface from the middle of _parse_header.

2. Both CED test guards

There are two, and each fails differently with 1.9.12:

  • neo/test/iotest/test_cedio.py replicated the old per-platform dispatch
    (import sonpy.linux.sonpy, …), which no longer exists → HAVE_SONPY = False → skipped.
  • neo/test/rawiotest/test_cedrawio.py used a bare import sonpy, which succeeds with
    1.9.12 → HAVE_SONPY = True → the test would run and fail with the AttributeError.

Both now delegate to _get_sonpy_namespace(), so "sonpy is usable" has one definition.

3. pyproject.toml — the test extra never installed sonpy

"sonpy;python_version<'3.10'",

with requires-python = ">=3.10". The marker cannot be satisfied, so sonpy is absent from
every CI run and both guards were moot regardless of how they were written. This is why the
breakage went unnoticed.

Changed to match where sonpy actually publishes usable wheels:

"sonpy; platform_system=='Windows' or python_version>='3.14'",

1.9.12 ships win_amd64 wheels for cp39–cp314, but manylinux and macosx only for cp314.
The marker is deliberately not just "sonpy": on Linux 3.10–3.13 pip would fall back to the
source distribution, which ships a Windows .pyd (verified: PE32+ executable (DLL) […] for MS Windows) and produces an unusable install.

The practical effect is that the automatic ubuntu-latest / Python 3.14 CI job will now
install sonpy and actually exercise CedRawIO.

Verification

The existing test suite goes from failing to passing. Windows, Python 3.12, sonpy 1.9.12,
against master (35cbce7):

pytest neo/test/rawiotest/test_cedrawio.py -v
unpatched FAILEDAttributeError: module 'sonpy' has no attribute 'lib' at cedrawio.py:72
patched PASSED (1.07 s) — test_read_all across all three spike2 entities

That covers both .smrx and the two .smr entities, so the reader is exercised end to end
and not just at import time.

Additionally:

  • Windows, Python 3.10–3.14, sonpy 1.9.12: sonpy.lib absent on all five; the compiled
    library reads spike2/m365_1sec.smrx correctly through the new namespace
    (GetOpenError() == 0, MaxChannels() == 101), confirming an import-path problem rather
    than a functional regression in sonpy.
  • Linux x86_64, Python 3.14, sonpy 1.9.12: the helper resolves to sonpy.sonpy; SonFile,
    DataType.Adc, DataType.AdcMark and MarkerFilter are all reachable, and
    CedRawIO.parse_header() runs to completion instead of raising.
  • Both guards checked in both directions: HAVE_SONPY is True with sonpy 1.9.12 present
    and False when sonpy cannot be imported.
  • The proposed marker evaluates True exactly where a usable wheel exists, across the six
    platform/version combinations in Neo's support range.
  • black --line-length 120 --check clean on all changed files.
  • After the three commits: pytest neo/test/rawiotest/test_cedrawio.py neo/test/iotest/test_cedio.py -v8 passed.

Not covered

macOS is untested. It is the one platform where the 1.9.12 wheel exists only for cp314, so
3.10–3.13 are excluded by the marker there. Happy to run it if someone has a machine.

Open questions

  1. Is platform_system=='Windows' or python_version>='3.14' acceptable, or would you rather
    keep the test extra minimal and add sonpy only to the CI workflow?
  2. Should the ced extra (line 101, currently a bare "sonpy") get the same marker for
    consistency? I left it alone to keep the diff focused.

AxelNoun and others added 3 commits July 30, 2026 18:52
sonpy 1.9.12 dropped the 'lib' namespace that cedrawio.py used, so every
sonpy.lib.* access raised AttributeError. Resolve the namespace once,
probing sonpy.lib, sonpy and sonpy.sonpy in turn: the last is needed on
Linux, where the 1.9.12 wheel ships an empty __init__.py.

Refs NeuralEnsemble#1890

Co-authored-by: Cursor <cursoragent@cursor.com>
test_cedio.py replicated the old per-platform sonpy dispatch and skipped
with 1.9.12; test_cedrawio.py guarded on a bare 'import sonpy', which
succeeds with 1.9.12 so the test would run and fail. Both now delegate to
_get_sonpy_namespace().

Refs NeuralEnsemble#1890

Co-authored-by: Cursor <cursoragent@cursor.com>
The test extra declared sonpy;python_version<'3.10' while the project
requires >=3.10, so sonpy was never installed and the CED tests never ran.
Target the platform/version combinations sonpy actually publishes wheels
for; the sdist ships a Windows .pyd and is not usable elsewhere.

Refs NeuralEnsemble#1890

Co-authored-by: Cursor <cursoragent@cursor.com>

@zm711 zm711 left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I like this overall, though I would prefer a tiny rewrite in the logic of the new function you've created. Your other concerns about the testing were on purpose from our perspective. When we no longer have active contributors (or if one of us doesn't have the familiarity/time) for an io we do a slow deprecation process where we slowly reduce testing (but keep it accessible for people) until it completely dies. Your work appears to revive thisio which means that we are happy to fully test things again within your constraints.

Comment thread neo/rawio/cedrawio.py Outdated
Comment on lines +55 to +58
try:
candidates.append(importlib.import_module("sonpy.sonpy"))
except ImportError:
pass

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I don't think this try-except is necessary. you could use importlib.util.find_spec to verify submodules without importing and without the try except (you just have to be careful of the logic to prevent errors before you've got to the deepest nesting. Then you can just do the import at the end with import_module.

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

Good call, thanks, pushed in a follow-up commit. Two details worth flagging:

  • import importlib doesn't pull in importlib.util, so the import is now explicit.
  • find_spec("sonpy.sonpy") imports the parent and raises ModuleNotFoundError if sonpy has no __path__. Guarding on that first keeps the failure on the explicit ImportError below rather than surfacing from find_spec.

Reordering also means the submodule is only imported when the first two candidates come up empty, i.e. the Linux >= 1.9.12 case, instead of eagerly on every platform. Verified against four fake sonpy layouts: right namespace on the three real ones, explicit ImportError on a package exposing nothing.

Per review: find_spec verifies sonpy.sonpy without importing it, so the
try/except goes away. Two guards are needed for that to hold: importlib.util
must be imported explicitly, and find_spec raises ModuleNotFoundError if
sonpy is not a package, so check __path__ first. Probing after the first two
candidates also means the submodule is only imported on Linux >= 1.9.12.

Refs NeuralEnsemble#1890

Co-authored-by: Cursor <cursoragent@cursor.com>
@AxelNoun

AxelNoun commented Aug 3, 2026

Copy link
Copy Markdown
Author

I like this overall, though I would prefer a tiny rewrite in the logic of the new function you've created. Your other concerns about the testing were on purpose from our perspective. When we no longer have active contributors (or if one of us doesn't have the familiarity/time) for an io we do a slow deprecation process where we slowly reduce testing (but keep it accessible for people) until it completely dies. Your work appears to revive thisio which means that we are happy to fully test things again within your constraints.

Thanks, that context helps and it reframes part of my description. I read the test extra marker and the two divergent guards as accidents; if the reduced testing was deliberate, then "this is why the breakage went unnoticed" is the wrong framing on my side. Happy to reword that section so it doesn't read as an oversight report.

On the revival: I'm happy to take CedRawIO on going forward. Ping me on anything CED and I'll pick it up.

Taking "within your constraints" as a green light on the two open questions:

  1. I'll keep sonpy; platform_system=='Windows' or python_version>='3.14' in the test extra. The practical effect is that the existing ubuntu-latest / 3.14 job installs sonpy and exercises the reader end to end, so coverage comes back without a new matrix entry.

  2. The ced extra is worth the same marker, I think. Today pip install neo[ced] on Linux 3.10 to 3.13 falls back to the sdist and installs a Windows .pyd, so users get an install that looks fine and fails later. With the marker they get nothing installed plus the explicit ImportError from _get_sonpy_namespace() at first use, which at least names the constraint. Say the word and I'll add it here, otherwise I'll keep this diff focused and open a separate PR.

sonpy only ships wheels for Windows, and for Linux and macOS from 3.14 on,
so neo[ced] silently resolves to nothing elsewhere. Say so, and point users
at Spike2RawIO for .smr files, which needs no sonpy.

Refs NeuralEnsemble#1890

Co-authored-by: Cursor <cursoragent@cursor.com>
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.

CedRawIO is broken with sonpy >= 1.9.12: the lib namespace no longer exists

2 participants