Skip to content

Comprehensive parser rewrite: correctness, robustness, and hygiene - #3

Merged
dambach merged 7 commits into
mainfrom
damien_doc-test-validationEntree
Apr 29, 2026
Merged

Comprehensive parser rewrite: correctness, robustness, and hygiene#3
dambach merged 7 commits into
mainfrom
damien_doc-test-validationEntree

Conversation

@dambach

@dambach dambach commented Apr 29, 2026

Copy link
Copy Markdown
Collaborator

Core parser fixes (A1–A2, B1–B3):

  • (A1) time_abs now strictly monotonic across blocks: offset += interval_s at boundaries
  • (A2) Per-cell NaN parsing: one bad cell doesn't drop the entire row
  • (B1) UTF-8 → latin-1 fallback encoding (no silent byte loss)
  • (B2) UserWarning when TimeFormat is not 'StartOfBlock'
  • (B3) Narrow exception handling in Interval_s parsing

Full per-block metadata segmentation (A3):

  • Single-pass block detection via header transitions + time resets
  • meta["blocks"]: list[dict] for per-block metadata
  • ChannelTitle/Range/TopValue/BottomValue removed from top-level meta

Bulk parsing rewrite (B4):

  • Replace per-cell float() loops with pd.read_csv + per-column coerce
  • ~320k rows/s on 500k×5 synthetic file (was O(n) per-cell)
  • Comment-row classification pre-pass to avoid re-scanning

Packaging hygiene (C1–C10):

  • Python ≥3.10 (was ≥3.8)
  • matplotlib as hard dependency (not optional)
  • Centralized tool config in pyproject.toml
  • Added [project.urls] for PyPI
  • Terse documentation: README 130→57 lines, CONTRIBUTING 220→39 lines
  • CLAUDE.md architecture guide for future contributors
  • New tests: block boundaries, encoding, time-format warning, bulk parsing
  • nbstripout hook for notebook output cells

Tests: 46→56 total (added boundary/encoding/format/bulk cases) All passing on Python 3.10+; ruff/black/isort clean.

Breaking changes from v0.1.2:

  • FileParsingError/NoDataError instead of ValueError on bad/missing data
  • time_abs values differ at block boundaries (now monotonic)
  • One bad cell → NaN, not whole row → comment
  • UserWarning on non-StartOfBlock exports

Core parser fixes (A1–A2, B1–B3):
- (A1) time_abs now strictly monotonic across blocks: offset += interval_s at boundaries
- (A2) Per-cell NaN parsing: one bad cell doesn't drop the entire row
- (B1) UTF-8 → latin-1 fallback encoding (no silent byte loss)
- (B2) UserWarning when TimeFormat is not 'StartOfBlock'
- (B3) Narrow exception handling in Interval_s parsing

Full per-block metadata segmentation (A3):
- Single-pass block detection via header transitions + time resets
- meta["blocks"]: list[dict] for per-block metadata
- ChannelTitle/Range/TopValue/BottomValue removed from top-level meta

Bulk parsing rewrite (B4):
- Replace per-cell float() loops with pd.read_csv + per-column coerce
- ~320k rows/s on 500k×5 synthetic file (was O(n) per-cell)
- Comment-row classification pre-pass to avoid re-scanning

Packaging hygiene (C1–C10):
- Python ≥3.10 (was ≥3.8)
- matplotlib as hard dependency (not optional)
- Centralized tool config in pyproject.toml
- Added [project.urls] for PyPI
- Terse documentation: README 130→57 lines, CONTRIBUTING 220→39 lines
- CLAUDE.md architecture guide for future contributors
- New tests: block boundaries, encoding, time-format warning, bulk parsing
- nbstripout hook for notebook output cells

Tests: 46→56 total (added boundary/encoding/format/bulk cases)
All passing on Python 3.10+; ruff/black/isort clean.

Breaking changes from v0.1.2:
- FileParsingError/NoDataError instead of ValueError on bad/missing data
- time_abs values differ at block boundaries (now monotonic)
- One bad cell → NaN, not whole row → comment
- UserWarning on non-StartOfBlock exports

Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
Copilot AI review requested due to automatic review settings April 29, 2026 09:26

Copilot AI 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.

Pull request overview

This PR rewrites the LabChart .txt parser for correctness and performance, introduces per-block metadata segmentation, and tightens packaging/CI hygiene (Python 3.10+, tool config, new tests/docs).

Changes:

  • Parser refactor: single-pass block segmentation, bulk read_csv parsing with per-cell NaN coercion, strict monotonic time_abs, encoding fallback, and more specific errors/warnings.
  • Metadata/API updates: meta["blocks"] per-block metadata, new custom exceptions, and a higher-level LabChartFile wrapper with plotting.
  • Project hygiene: pyproject tool centralization, CI + pre-commit, docs trim, changelog, and expanded test suite.

Reviewed changes

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

Show a summary per file
File Description
tests/test_parser.py Adds parser regression/edge-case tests (blocks, monotonic time, encoding, warnings, bulk path).
tests/test_core.py Adds high-level LabChartFile API tests (channels, blocks, slicing, plotting).
tests/conftest.py Adds shared pytest fixtures for example data paths.
tests/init.py Defines test package docstring.
src/labchart_parser/parser.py Major parser rewrite: block segmentation, bulk parsing, encoding fallback, monotonic time_abs, new metadata shape and warnings.
src/labchart_parser/exceptions.py Introduces custom exception classes used across the parser/core API.
src/labchart_parser/core.py Adds/expands LabChartFile wrapper API and a plotting helper.
src/labchart_parser/init.py Exposes __version__ via package metadata and updates __all__.
pyproject.toml Updates Python floor/dependency constraints and centralizes tool configuration + URLs.
examples/example_usage.py Updates example usage to use plot_channel() and cleaned-up narrative.
README.md Updates badges, installation notes, and quick-start documentation.
CONTRIBUTING.md Adds streamlined contribution and workflow guidance.
CODE_OF_CONDUCT.md Adds Contributor Covenant code of conduct.
CLAUDE.md Adds architecture/maintenance notes for contributors and tooling.
CHANGELOG.md Adds changelog and documents the set of user-visible changes.
.pre-commit-config.yaml Adds formatting/linting and nbstripout hooks.
.github/workflows/ci.yml Adds CI matrix for tests/lint/build on Python 3.10–3.12 across OSes.

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

Comment thread src/labchart_parser/parser.py Outdated
Comment thread src/labchart_parser/parser.py Outdated
Comment thread tests/test_parser.py Outdated
Comment thread pyproject.toml Outdated
[project]
name = "labchart_parser"
version = "0.1.1"
version = "0.1.2"

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

@copilot apply changes based on this feedback

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

@copilot apply changes based on this feedback

Comment thread src/labchart_parser/core.py Outdated
Comment thread src/labchart_parser/core.py Outdated
Comment on lines +8 to +9
import matplotlib.pyplot as plt
import pandas as pd

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

@copilot apply changes based on this feedback

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

@copilot apply changes based on this feedback

dambach and others added 2 commits April 29, 2026 11:33
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
@dambach

dambach commented Apr 29, 2026

Copy link
Copy Markdown
Collaborator Author

@copilot apply changes based on the comments in this thread

dambach and others added 2 commits April 29, 2026 11:34
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
@dambach

dambach commented Apr 29, 2026

Copy link
Copy Markdown
Collaborator Author

@copilot resolve the merge conflicts in this pull request

Copilot AI commented Apr 29, 2026

Copy link
Copy Markdown
Contributor

@copilot apply changes based on the comments in this thread

Applied all open review comments in commit 1fb65ce:

  • core.py:8-9matplotlib.pyplot is now imported lazily inside plot_channel(), so importing labchart_parser no longer triggers a matplotlib import (avoids overhead and headless-backend issues).
  • core.py:45blocks was already returning [int(b) for b in ...] (Python ints) after the remote rebase.
  • pyproject.toml:7 — version bumped to 0.2.0 to reflect the breaking API/behavior changes; CHANGELOG.md updated with a [0.2.0] section.
  • tests/test_parser.py:50 — already using pd.api.types.is_integer_dtype(df["block"]) — no change needed.

…ationEntree

# Conflicts:
#	README.md
#	src/labchart_parser/parser.py

Co-authored-by: dambach <20612130+dambach@users.noreply.github.com>

Copilot AI commented Apr 29, 2026

Copy link
Copy Markdown
Contributor

@copilot resolve the merge conflicts in this pull request

Resolved in commit 221ea98. Two files had conflicts:

  • README.md — kept the PR's more precise description ("Parses ADInstruments LabChart .txt exports into a pandas DataFrame with blocks, continuous time, and comments.").
  • src/labchart_parser/parser.py — kept the richer module-level docstring from main; merged the function docstring by adopting main's numpy-style Returns section while replacing its outdated ValueError in Raises with the correct FileNotFoundError, FileParsingError, and NoDataError types from this PR.

All 56 tests pass.

@dambach
dambach merged commit 802de1f into main Apr 29, 2026
10 checks passed
@dambach
dambach deleted the damien_doc-test-validationEntree branch April 29, 2026 09:47
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.

3 participants