Skip to content

Add segment annotation mode to edit_physio() - #108

Open
me-pic wants to merge 8 commits into
physiopy:masterfrom
me-pic:ENH/annotate
Open

me-pic wants to merge 8 commits into
physiopy:masterfrom
me-pic:ENH/annotate

Conversation

@me-pic

@me-pic me-pic commented Jul 31, 2026 •

Copy link
Copy Markdown
Contributor

Closes #

[WIP]
I modified the peakdet codebase a moment ago to be able to annotate segments (save the onset and offset of segments) during the manual editing (operations.edit_physio) for my personal use case. I'm doing that PR in case that could be a feature useful for others!

Proposed Changes

  • Add annotate_segments() function and modify existing functions in editor.py to be able to annotate segments in the interactive viewer
  • Add a segments property to be able to store the onset and offset of selected segments in the segments attribute of a Physio instance, and in the metadata. Output example:
>>> data.segments
array([( 508., 1086.), (1661., 1987.)],
      dtype=[('onset', '<f8'), ('offset', '<f8')])
  • Store the onset and offset of the segments in Physio object history. Output example:
>>> data.history
[('peakfind_physio', {}), ('annotate_segments', {'segment': [(np.float64(508.0), np.float64(1086.0)), (np.float64(1661.0), np.float64(1987.0))]})]
  • Modify the docs to add information relative to the 'annotation mode'

A few comments/questions:

  • I had to modify the .pre-commit-config.yaml file and specify the python version to be able to run the pre-commit hooks. Ideally I guess the .pre-commit-config.yaml would not include it, so if anybody wants to discuss the error I got before that change, it would be great!
  • I had to manually skip the check-added-large-files hook to be able to commit the physio_edit_segment.gif for the documentation (the file size is smaller than the other gif already included i.e. physio_edit.gif).
  • I wasn't able to build the documentation locally as is (via make html in the docs/ subdirectory). There was a problem with L118 in conf.py. I ended up just commenting that line for the sake of testing the rendering of the modified doc. I would have to investigate a bit more on that, but it's also been a while since the last rtd build (5 years ago) and I'm wondering if the problem is not only local.
  • For the annotation itself, the current behavior of that mode doesn't allow for overlapping segment. If you have a segment and you select one that overlap, the previous segment will be removed and only the last segment will remain. Happy to make the changes to allow such behavior depending on what would be the most useful to others! EDIT: This has been changed to allow overlapping segments

Change Type

  • bugfix (+0.0.1)
  • minor (+0.1.0)
  • major (+1.0.0)
  • refactoring (no version update)
  • test (no version update)
  • infrastructure (no version update)
  • documentation (no version update)
  • other

Checklist before review

  • I added everything I wanted to add to this PR.
  • [Code or tests only] I wrote/updated the necessary docstrings.
  • [Code or tests only] I ran and passed tests locally.
  • [Documentation only] I built the docs locally.
  • My contribution is harmonious with the rest of the code: I'm not introducing repetitions.
  • My code respects the adopted style, especially linting conventions.
  • The title of this PR is explanatory on its own, enough to be understood as part of a changelog.
  • I added or indicated the right labels.
  • I added information regarding the timeline of completion for this PR.
  • Please, comment on my PR while it's a draft and give me feedback on the development!

@github-actions github-actions Bot added the Internal Changes affect the internal API. It doesn't increase the version, but produces a changelog label Jul 31, 2026
@me-pic

me-pic commented Sep 24, 2026

Copy link
Copy Markdown
Contributor Author

TO CHANGE: Allow overlapping segments

@smoia smoia added Documentation This issue or PR is about the documentation Minormod This PR generally closes an "Enhancement" issue. It increments the minor version (0.+1.0) and removed Internal Changes affect the internal API. It doesn't increase the version, but produces a changelog labels Sep 24, 2026
me-pic and others added 4 commits September 29, 2026 10:55
Modify the behavior of check_segment() to allow the possibility of overlapping segments.
Modify the behavior of check_segment() to allow the possibility of overlapping segments.
@github-actions github-actions Bot added the Internal Changes affect the internal API. It doesn't increase the version, but produces a changelog label Sep 29, 2026
@me-pic

me-pic commented Sep 29, 2026

Copy link
Copy Markdown
Contributor Author

I've changed the code to allow overlapping segments, and modify the documentation to reflect that new behavior.

One thing is that it is currently not possible to remove segments. So if a Physio object has been edited and segments were "annotated" (meaning saving the onset and offset of segments), and the edit_physio() is called again on this object, it is not possible to remove the segments. So I was thinking to add that option. The propose changes:

  • Changing the command to add segments in the "annotation mode" (ctrl+a) to right click + drag (instead of left clik + drag) to be consistent with the command to add peaks in the "edit mode" (ctrl+e)
  • Add a delete segment command (left click + drag) in the "annotation mode". The segment that is overlapping with the window that is selected with that command would be deleted. So for example, if we have a segment with (onset, offset) = (0, 100), and we want to delete it, we can just select a window with the (onset, offset) anywhere within that (0,100) segment.

One problem with the second proposed change is that if we have two overlapping segments with one complete within the other. Example, segment1 (onset, offset) = (0, 100); segment2 (onset, offset) = (25, 50). In that case, I think that if one select a window within (25, 50) to be deleted, only segment2 should be deleted. If the selected window is for example (30, 60) then both segments should be deleted.

@smoia WDYT?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Documentation This issue or PR is about the documentation Internal Changes affect the internal API. It doesn't increase the version, but produces a changelog Minormod This PR generally closes an "Enhancement" issue. It increments the minor version (0.+1.0)

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants