Skip to content

feat: enhance sync channel plotting and fix tox coverage path#255

Open
Thashira wants to merge 2 commits into
neuroinformatics-unit:mainfrom
Thashira:main
Open

feat: enhance sync channel plotting and fix tox coverage path#255
Thashira wants to merge 2 commits into
neuroinformatics-unit:mainfrom
Thashira:main

Conversation

@Thashira

@Thashira Thashira commented Mar 22, 2026

Copy link
Copy Markdown

Before submitting a pull request (PR), please read the contributing guide.

Please fill out as much of this template as you can, but if you have any problems or questions, just leave a comment and we will help out :)

Description

What is this PR

  • Bug fix
  • [x ] Addition of a new feature
  • [ x] Other

Why is this PR needed?

This PR addresses the TODO in _raw_run.py regarding the improvement of sync channel visualization. Currently, plotting the sync channel lacks time-range support, which is critical for researchers to verify digital trigger alignments in long electrophysiological recordings without incurring memory overhead or UI freezes.

What does this PR do?

1.Time-Range Support: Adds a time_range parameter to plot_sync_channel, allowing specific windowing (defaulting to 0-10s for safety).

2.Physical Units: Introduces a use_seconds flag to plot against a time axis (s) rather than sample indices, improving interpretability.

3.CI Maintenance: Updates tox.ini to point coverage to the correct spikewrap module (formerly swc_epys).

4.UI Improvements: Standardizes plot aesthetics (labels, grid, and coloring) as requested in the source code TODO.

References

Please reference any existing issues/PRs that relate to this PR.

1.Addresses internal TODO in spikewrap/structure/_raw_run.py.

How has this PR been tested?

1.Manual Verification: Tested with the internal sub-001 example dataset to ensure time-to-sample indexing accurately reflects the sampling frequency (fs).

2.Integration Testing: Ran pytest tests/test_integration/test_sync.py and tests/test_integration/test_preprocessing.py to ensure core workflows remain intact.

3.Linting: All changes passed pre-commit hooks (Ruff, Black, Mypy).

Please explain how any new code has been tested, and how you have ensured that no existing functionality has changed.

Is this a breaking change?

If this PR breaks any existing functionality, please explain how and why.

No.

While the function signature changed, I implemented default values (time_range=None, use_seconds=False) that ensure all existing calls in the test suite and user scripts remain fully compatible.

Does this PR require an update to the documentation?

If any features have changed, or have been added. Please explain how the
documentation has been updated.

Yes, the docstrings for Session.plot_sync_channel and SeparateRawRun.plot_sync_channel have been updated to reflect the new parameters.

Checklist:

  • [ x] The code has been tested locally
  • Tests have been added to cover all new functionality
  • [ x] The documentation has been updated to reflect any changes
  • [ x] The code has been formatted with pre-commit

Copilot AI review requested due to automatic review settings March 22, 2026 16:39

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Pull request overview

This PR aims to improve sync-channel visualization for long recordings by adding windowed plotting options, and fixes CI coverage configuration to target the correct module.

Changes:

  • Add time_range and use_seconds parameters to Session.plot_sync_channel and delegate them to the run-level plotter.
  • Update RawRun.plot_sync_channel plotting to support windowing and improved aesthetics.
  • Fix tox.ini coverage target from swc_epys to spikewrap.

Reviewed changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated 6 comments.

File Description
tox.ini Updates pytest coverage target to the spikewrap package.
spikewrap/structure/session.py Extends session-level sync plotting API to pass through windowing/unit parameters.
spikewrap/structure/_raw_run.py Implements windowed plotting logic and refreshed plot styling for the sync channel.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread spikewrap/structure/session.py
Comment thread spikewrap/structure/_raw_run.py Outdated
Comment thread spikewrap/structure/_raw_run.py
Comment thread spikewrap/structure/_raw_run.py
Comment thread spikewrap/structure/session.py
Comment thread spikewrap/structure/session.py
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.

2 participants