Skip to content

Add string sample logs - #116

Open
alexhroom wants to merge 4 commits into
mainfrom
70-string-sample-logs
Open

alexhroom wants to merge 4 commits into
mainfrom
70-string-sample-logs

Conversation

@alexhroom

@alexhroom alexhroom commented Sep 11, 2026 •

Copy link
Copy Markdown
Collaborator

This PR adds the ability to filter sample logs which are strings. Fixes #70.

Also fixes a bug where sample log filters are filtered wrong in output files as apply_filters ignores whether the time filters are include or exclude, and adds some validation for sample logs when the filter is created rather than when it is calculated.

also now sorts sample log times. fixes #100

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.

🟡 Changes recommended

Missing-value filters can return all data, and transition timestamps are incorrectly included.

Get a fresh assessment by requesting another Copilot review.

Pull request overview

Adds string sample-log filtering, validation, and corrected filter handling in histogram/output generation.

Changes:

  • Supports case-insensitive string predicates and HDF5 string logs.
  • Sorts and applies include/exclude ranges correctly.
  • Adds tests, plotting support, and documentation.
File summaries
File Description
src/test_utils.rs Adds mock sample-log creation.
src/stats.rs Propagates log-filter errors.
src/interface.rs Exposes string filtering.
src/filters/mod.rs Exports predicates.
src/filters/api.rs Adds string predicates and errors.
src/data/save/sample_logs.rs Saves and filters string logs.
src/data/sample_logs.rs Loads, filters, and sorts log ranges.
src/data/nexus_data.rs Returns string logs to Python.
src/batch_interface.rs Validates and applies string filters.
MNeuEventLib/plotting.py Plots categorical logs.
docs/source/how-to/filtering.ipynb Documents text filtering.
Review details
  • Files reviewed: 11/11 changed files
  • Comments generated: 2
  • Review effort level: Balanced

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

Comment thread src/batch_interface.rs Outdated
Comment thread src/data/sample_logs.rs Outdated
@alexhroom alexhroom added the feature New feature or request label Sep 21, 2026
@github-actions github-actions Bot added the Has Conflicts This PR has a merge conflict. label Sep 21, 2026
@github-actions

Copy link
Copy Markdown

👋 Hi, @alexhroom,

Conflicts have been detected against the base branch. Please rebase your branch against the base branch.


This message is automatically generated by prince-chrismc/label-merge-conflicts-action so don't hesitate to report issues/improvements there.

@alexhroom
alexhroom force-pushed the 70-string-sample-logs branch from 41cb2e9 to f70f6ad Compare September 21, 2026 14:52
@github-actions github-actions Bot removed the Has Conflicts This PR has a merge conflict. label Sep 21, 2026
@github-actions github-actions Bot added the Has Conflicts This PR has a merge conflict. label Sep 30, 2026
@github-actions

Copy link
Copy Markdown

👋 Hi, @alexhroom,

Conflicts have been detected against the base branch. Please rebase your branch against the base branch.


This message is automatically generated by prince-chrismc/label-merge-conflicts-action so don't hesitate to report issues/improvements there.

@alexhroom
alexhroom force-pushed the 70-string-sample-logs branch from f70f6ad to ba100d0 Compare October 1, 2026 15:03
@github-actions github-actions Bot added Has Conflicts This PR has a merge conflict. and removed Has Conflicts This PR has a merge conflict. labels Oct 1, 2026
@github-actions

github-actions Bot commented Oct 2, 2026

Copy link
Copy Markdown

👋 Hi, @alexhroom,

Conflicts have been detected against the base branch. Please rebase your branch against the base branch.


This message is automatically generated by prince-chrismc/label-merge-conflicts-action so don't hesitate to report issues/improvements there.

@alexhroom
alexhroom force-pushed the 70-string-sample-logs branch from ba100d0 to 0ad6b9d Compare October 2, 2026 12:52
@github-actions github-actions Bot removed the Has Conflicts This PR has a merge conflict. label Oct 2, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

feature New feature or request

Projects

None yet

Development

Successfully merging this pull request may close these issues.

SampleLog::apply_filters() assumes time filters are in chronological order, which they aren't String sample logs

2 participants