Skip to content

fix: mkdocs description + staged error handling in plot() (#56, #50)#57

Open
tschm wants to merge 2 commits into
alihaskar:masterfrom
tschm:fix-issues-56-54-50
Open

fix: mkdocs description + staged error handling in plot() (#56, #50)#57
tschm wants to merge 2 commits into
alihaskar:masterfrom
tschm:fix-issues-56-54-50

Conversation

@tschm

@tschm tschm commented Jul 20, 2026

Copy link
Copy Markdown
Contributor

Addresses three quality issues.

#56 — Replace placeholder mkdocs site_description

mkdocs.yml carried the scaffold placeholder Add your description here.. Set it to the real description already present in pyproject.toml (single source of truth):

High-performance financial charting library for OHLC data visualization with technical indicators

#50 — Narrow broad except Exception in plot()

The plot() body was wrapped in a single broad except Exception, collapsing input-validation failures and server/runtime failures into one generic error. Split into:

  • except DataValidationError{"status": "error", "stage": "validation", ...} — fixable by correcting input arrays.
  • except Exception{"status": "error", "stage": "server", ...} — server startup / browser / runtime failures.

The error key is preserved for backward compatibility. test_plot_with_invalid_data now asserts stage == "validation".

#54 — Failing test_websocket_endpoint_added

The test already carries the defensive getattr(route, "path", None) guard and passes; closing that issue separately as already-fixed.

Verification

Full suite: 94 passed, exit 0.

Closes #56
Closes #50

🤖 Generated with Claude Code

…haskar#56, alihaskar#50)

- mkdocs.yml: replace placeholder site_description with the real
  description from pyproject.toml (alihaskar#56)
- interface.py: split the broad except in plot() so DataValidationError
  is reported with stage="validation" (fixable input) separately from
  server/runtime failures reported with stage="server" (alihaskar#50)
- test_interface.py: assert the validation stage is reported

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Copilot AI review requested due to automatic review settings July 20, 2026 07:34

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 improves project metadata and makes the Python plot() API’s error reporting more actionable by distinguishing user-fixable input validation failures from server/runtime failures.

Changes:

  • Replaces the MkDocs scaffold placeholder site_description with the real project description (matching pyproject.toml).
  • Splits plot() error handling into DataValidationError vs generic runtime errors and annotates error results with a stage.
  • Updates the invalid-data plot test to assert stage == "validation".

Reviewed changes

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

File Description
mkdocs.yml Replaces placeholder site_description with the real project description for published docs metadata.
src/pycharting/api/interface.py Adds staged error handling to plot() by catching DataValidationError separately and returning stage in error results.
tests/pycharting/api/test_interface.py Tightens the invalid-data test to verify validation errors are surfaced with stage: "validation".

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

Splitting the broad except in plot() left the generic (server/runtime)
except branch unexercised once validation errors took their own branch,
dropping interface.py coverage below 100%. Add a test that forces a
ChartServer startup failure and asserts stage="server".

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
@tschm

tschm commented Jul 20, 2026

Copy link
Copy Markdown
Contributor Author

Merged into #58

The commits on this branch (fix-issues-56-54-50) have been merged into #58 (fix-test-layout-55), so that PR now also carries these fixes:

No conflicts on merge. #58 supersedes this PR — recommend closing this one in favour of #58 (or merging #57 first, then #58).

🤖 Generated with Claude Code

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.

Replace placeholder mkdocs site_description with real project description Narrow broad except Exception in plot()

2 participants