Conversation
- Add missing type imports (F821): - AnalysisDataSet in analysis.py - AnalysisSpecification in data_preparation.py and sds_detector.py - Study in process_dataframe.py (via TYPE_CHECKING) - Fix line length violations (E501): - Split long description strings in examples/sds_detection_demo.py - Extract inline ternaries to variables in plotting/plotter.py - Apply ruff auto-fixes: - Convert if-else blocks to ternary expressions (SIM108) - Remove unnecessary f-string prefixes (F541) - Sort imports (I001) - Remove unused imports (F401) - Remove quoted type annotations (UP037) - Fix remaining style issues: - Combine nested if statements (SIM102) in analysis_specification.py - Rename unused loop variable to _name (B007) in plotter.py - Suppress complexity warnings (C901) for chart_table() and plot_effects() methods that exceed max-complexity but are intentionally comprehensive 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com>
Replace scipy.special.loggamma with stdlib math.lgamma for calculating the c4 SPC constant. This eliminates a ~30MB dependency for a single function call. scipy is still used optionally in plot_residuals() for normal distribution functions (histogram overlay, Q-Q plots). Added helpful ImportError message when scipy is needed but not installed. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com>
Move the residuals availability check before the scipy import in plot_residuals(). This gives users a helpful error message about missing residuals instead of asking them to install scipy when they don't have residuals to plot anyway. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com>
Collaborator
Author
|
@cnicholas also see Issue #48 . The remainder of the scipy dependencies in plotting can be easily eliminated, but as you're not even running those tests due to bypass logic, I'd recommend getting your green check wtih this PR and then tackling that one which will require some testing updates. |
Owner
|
Thanks for the great work on linting fixes, Ben! I've cherry-picked your ruff fixes commit (45b7845) into main as 4a267ec with co-author attribution. We had already fully removed scipy (with pure Python implementations for the normal distribution functions), so I skipped those commits to avoid conflicts. Closing this PR since the valuable changes have been incorporated. Thanks for contributing! |
Owner
|
Closing - changes cherry-picked into main. Thanks again! |
Owner
|
Thanks man - PR merged, I removed scipy last night and replaced with base
functionality. Also addressed issue #48 on the testing for plotting.
Claude got lazy on me...
…On Mon, Dec 8, 2025 at 6:24 AM Ben Ogorek ***@***.***> wrote:
*baogorek* left a comment (cnicholas/processbehavior#47)
<#47 (comment)>
@cnicholas <https://github.com/cnicholas> also see Issue #48
<#48> . The remainder
of the scipy dependencies in plotting can be easily eliminated, but as
you're not even running those tests due to bypass logic, I'd recommend
getting your green check wtih this PR and then tackling that one which will
require some testing updates.
—
Reply to this email directly, view it on GitHub
<#47 (comment)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/ABFYGJVV6MSDOIG5TO2XSJ34AVNVLAVCNFSM6AAAAACOLTE4HSVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZTMMRWGQZDCNRQGM>
.
You are receiving this because you were mentioned.Message ID:
***@***.***>
|
cnicholas
added a commit
that referenced
this pull request
Dec 9, 2025
Cherry-picked from PR #47 by Ben Ogorek (baogorek) - Add missing type imports (F821): AnalysisDataSet, AnalysisSpecification, Study - Fix line length violations (E501) in examples and plotter - Apply ruff auto-fixes: ternary expressions, import sorting, unused imports - Combine nested if statements (SIM102) - Rename unused loop variables (B007) - Remove unnecessary f-string prefixes (F541) - Remove quoted type annotations (UP037) Co-authored-by: Ben Ogorek <baogorek@gmail.com>
21 tasks
31 tasks
cnicholas
added a commit
that referenced
this pull request
Aug 24, 2026
… the truth ## Summary - **What:** Phase-0 release prep — Python 3.10 floor, a repaired notebook gate, and four places where the docs described an API that does not exist. - **Why:** These are the blockers a first-time visitor would hit. 3.9 went EOL in October 2025; half the published tutorials were known-stale; `result.effects` documented two keys the library has never produced. - **Scope:** packaging metadata, CI matrices, docs TOC, docstrings. No runtime behaviour changes. ## Behaviour changes - Python floor is now 3.10. The 3.9-conditional pins in `[test]`/`[lint]` are gone with it, and `[tool.mypy] python_version` moves to 3.10 — current mypy rejects "3.9" outright, so type-checking has been silently unrunnable. - `hatchling>=1.27`: the PEP 639 `license`/`license-files` fields this project already uses need it. The old `>=1.21` floor understated the requirement and would emit wrong metadata in a pinned build environment. ## The notebook gate was broken `ci.yml` listed `docs/tutorials/calibration.ipynb`, deleted in 91b4afe (July). The step could not have been passing. Meanwhile `coffee-shop.ipynb` — the flagship "complete story" tutorial — was in neither the gate nor the stale-tutorial excuse list, so the one notebook most likely to be read was the one least likely to be verified. Both fixed; all six allowlisted notebooks now execute clean locally. The six stale tutorials leave the docs TOC rather than sit in it advertising an old API. They stay on disk for rewriting. `docs/intro.md` no longer sends new readers to a stale notebook from its Quick Links, and the tutorial index table now matches what is actually published. ## Docs that described a different library - `result.effects` documented `'k_effects'` / `'t_effects'`. Neither key has ever existed. The real keys are named after the user's own factor columns, plus `main_effect`, `time`, `<factor>_MEs`, and `factor_interaction_effects` — now documented, including the namespace collision a factor named `time` would cause. Same fix in the two user-guide pages that copied the claim. - README: `pb.cols` is instance-level (`ProcessBehavior(df).cols`). - README: the residual example charted `X` with `value='R4'`, which is invalid on every design state — `('X','R4')` is not in `residual_charts` for ADS 1, 2 or 3. Corrected to `Xbar`, with a pointer to `study.residual_charts` and `why_not()` for the pairs that are refused. ## Also - `ci.yml` cited issue #77 for the mypy re-enable; #77 explicitly defers it to #47. - Default pytest timeout (300s, thread method) — `pytest-timeout` was a declared dependency that nothing configured, so a hang would block the matrix until GitHub's 6h kill. - Classifiers gain `Typing :: Typed`, `OS Independent`, `3 :: Only`; keywords gain `xmr`, `control-chart`, `process-behavior-chart`. - `.gitignore` covers `.claude/` and `docs/review/`. - CLAUDE.md: `make_sds` is `make_design`; `DataPrepConfig`/`AnalysisSpecification` are gone (it is `FormulationSpec` → `ChartRequest`); "attempted" cells reworded to "attempted but empty", which is what the detector actually does. ## Tests - `pytest tests/ -m "not slow"` — green. - `pytest --nbmake` over the corrected allowlist — 6/6 pass.
cnicholas
added a commit
that referenced
this pull request
Aug 24, 2026
…ames redirect
## Summary
- **What:** `result.plot()` now really exposes the plotly API it advertises,
`to_excel` stops writing a file it never mentions, Minitab/Wheeler chart
spellings get a redirect instead of a shrug, and mypy runs again.
- **Why:** four more places where the library's promise and its behaviour
disagreed.
## The plot wrapper promised the plotly API and delivered six methods
`ControlChartFigure`'s docstring said "maintaining full access to Plotly's
API". In fact `write_html`, `write_image`, `update_traces`, `add_hline` — the
things every plotly user reaches for first — raised AttributeError, and the
`.figure` escape hatch appeared nowhere in that error.
Now `__getattr__` delegates to the wrapped figure, with `__dir__` unioned so
completion finds both surfaces. Dunder names are refused rather than delegated:
letting `__deepcopy__`/`__reduce__` resolve to bound methods of the *figure*
makes copy, pickle and IPython display behave erratically, and `_fig` itself is
refused so an unpickle that runs `__getattr__` before the instance dict exists
cannot recurse forever. `write_html`/`write_image` are defined explicitly (not
left to fall through) so the latter keeps the error translation below.
## "Install kaleido" when kaleido was installed
`save_image` classified failures by substring-matching "kaleido" in the message,
so kaleido-present-but-no-browser — a completely different fix — was reported as
"Image export requires kaleido". People were told to reinstall a package they
already had.
`_translate_image_error` now separates the three: missing package (ImportError,
naming the extra), missing browser (RuntimeError, naming `plotly_get_chrome`),
and everything else (re-raised untouched, because not every failure here is
about the export backend).
## to_excel wrote a file it never mentioned
`export_html` defaulted to True, so `to_excel('analysis.xlsx')` also emitted a
standalone plotly document — multi-megabyte, unmentioned by the call, and
unreported by the return (there wasn't one). The workbook then carried an
"INTERACTIVE CHARTS" note promising those files.
`export_html` now defaults to False, `export()`/`to_excel()` return the list of
every path written, and the sheet note appears only when HTML was actually
written, naming the real filenames. The HTML step runs before the workbook is
written, since the sheet has to name files that exist. Per-chart image failure
also stops claiming to be a kaleido problem when it isn't, and a missing
kaleido short-circuits the remaining charts instead of logging N identical
warnings.
## Chart names an analyst actually types
`'xmr'` got a bespoke redirect; `'IMR'`, `'I-MR'`, `'Xbar-S'` — the Minitab
spellings this project's own README teaches as equivalent — got a bare "Unknown
chart". Now they all redirect with the spelling to use, and unrecognised names
get a difflib "Did you mean 'Xbar'?" at the two raise sites that know a chart
was meant.
Deliberately a redirect, not a silent alias: `XmR`/`IMR` name a chart *pair*, so
mapping either to one chart would guess which half was wanted. And deliberately
no did-you-mean inside `normalize_chart_name` — that function must pass unknown
names through, because stratum names flow through it and it cannot tell a typo
from a stratum.
`'xmr'` also raised a raw `ValueError` while its siblings raised
`ValidationError`; both are `ValidationError` now (which subclasses ValueError,
so existing handlers are unaffected).
## mypy was not running at all
`python_version = "3.9"` is rejected outright by current mypy, so type-checking
had been silently dead — `py.typed` was shipping annotations nothing verified.
3.10 does not fix it either: numpy's bundled stubs use 3.12 `type` statements
and mypy aborts on `numpy/__init__.pyi` before reaching our code. Set to 3.12
with the tradeoff written down — this is the stub-parsing version, not the
support floor, and ruff's `py310` target plus the 3.10 CI leg cover what it
stops catching.
With it running: 16 pre-existing errors in analysis.py (the TypedDict work
tracked in #47), and one I had just introduced — `_export_html_charts` still
annotated `-> None` while returning a list. Caught on the first run, which is
the argument for the whole exercise. Three more modules join the strict
per-module list (types, process_behavior, plotting.control_chart).
## Ruff
The `target-version` bump to py310 in the previous commit activated rules that
had never applied, leaving `ruff check .` — a hard CI gate — with 41 errors.
Fixed: PEP 604 annotations, sorted imports, yoda conditions, and `strict=False`
on every bare `zip()` (explicitly the behaviour-preserving choice; `strict=True`
would turn silent truncation into a runtime error, which is a decision for the
call sites, not a lint sweep).
## Docs
- excel-export: openpyxl and kaleido were described as automatically installed.
They are extras — that is the whole point of the dependency boundary.
- Five pages linked into tutorials that the previous commit pulled from the TOC.
## Tests
- `pytest tests/ -m "not slow"` — 2170 passed, 6 skipped.
- All six allowlisted notebooks execute clean.
- `ruff check .` clean; `mypy processbehavior` at its pre-existing baseline.
- New `tests/test_control_chart_figure.py` (delegation, dunder guard, copy
safety, the three error-translation branches) and `TestExportWritesWhatItSays`
(which asserts on-disk files exactly equal the reported list).
cnicholas
added a commit
that referenced
this pull request
Sep 1, 2026
Cherry-picked from PR #47 by Ben Ogorek (baogorek) - Add missing type imports (F821): AnalysisDataSet, AnalysisSpecification, Study - Fix line length violations (E501) in examples and plotter - Apply ruff auto-fixes: ternary expressions, import sorting, unused imports - Combine nested if statements (SIM102) - Rename unused loop variables (B007) - Remove unnecessary f-string prefixes (F541) - Remove quoted type annotations (UP037) Co-authored-by: Ben Ogorek <baogorek@gmail.com>
cnicholas
added a commit
that referenced
this pull request
Sep 1, 2026
… the truth ## Summary - **What:** Phase-0 release prep — Python 3.10 floor, a repaired notebook gate, and four places where the docs described an API that does not exist. - **Why:** These are the blockers a first-time visitor would hit. 3.9 went EOL in October 2025; half the published tutorials were known-stale; `result.effects` documented two keys the library has never produced. - **Scope:** packaging metadata, CI matrices, docs TOC, docstrings. No runtime behaviour changes. ## Behaviour changes - Python floor is now 3.10. The 3.9-conditional pins in `[test]`/`[lint]` are gone with it, and `[tool.mypy] python_version` moves to 3.10 — current mypy rejects "3.9" outright, so type-checking has been silently unrunnable. - `hatchling>=1.27`: the PEP 639 `license`/`license-files` fields this project already uses need it. The old `>=1.21` floor understated the requirement and would emit wrong metadata in a pinned build environment. ## The notebook gate was broken `ci.yml` listed `docs/tutorials/calibration.ipynb`, deleted in 4431e56 (July). The step could not have been passing. Meanwhile `coffee-shop.ipynb` — the flagship "complete story" tutorial — was in neither the gate nor the stale-tutorial excuse list, so the one notebook most likely to be read was the one least likely to be verified. Both fixed; all six allowlisted notebooks now execute clean locally. The six stale tutorials leave the docs TOC rather than sit in it advertising an old API. They stay on disk for rewriting. `docs/intro.md` no longer sends new readers to a stale notebook from its Quick Links, and the tutorial index table now matches what is actually published. ## Docs that described a different library - `result.effects` documented `'k_effects'` / `'t_effects'`. Neither key has ever existed. The real keys are named after the user's own factor columns, plus `main_effect`, `time`, `<factor>_MEs`, and `factor_interaction_effects` — now documented, including the namespace collision a factor named `time` would cause. Same fix in the two user-guide pages that copied the claim. - README: `pb.cols` is instance-level (`ProcessBehavior(df).cols`). - README: the residual example charted `X` with `value='R4'`, which is invalid on every design state — `('X','R4')` is not in `residual_charts` for ADS 1, 2 or 3. Corrected to `Xbar`, with a pointer to `study.residual_charts` and `why_not()` for the pairs that are refused. ## Also - `ci.yml` cited issue #77 for the mypy re-enable; #77 explicitly defers it to #47. - Default pytest timeout (300s, thread method) — `pytest-timeout` was a declared dependency that nothing configured, so a hang would block the matrix until GitHub's 6h kill. - Classifiers gain `Typing :: Typed`, `OS Independent`, `3 :: Only`; keywords gain `xmr`, `control-chart`, `process-behavior-chart`. - `.gitignore` covers `.claude/` and `docs/review/`. - CLAUDE.md: `make_sds` is `make_design`; `DataPrepConfig`/`AnalysisSpecification` are gone (it is `FormulationSpec` → `ChartRequest`); "attempted" cells reworded to "attempted but empty", which is what the detector actually does. ## Tests - `pytest tests/ -m "not slow"` — green. - `pytest --nbmake` over the corrected allowlist — 6/6 pass.
cnicholas
added a commit
that referenced
this pull request
Sep 1, 2026
…ames redirect
## Summary
- **What:** `result.plot()` now really exposes the plotly API it advertises,
`to_excel` stops writing a file it never mentions, Minitab/Wheeler chart
spellings get a redirect instead of a shrug, and mypy runs again.
- **Why:** four more places where the library's promise and its behaviour
disagreed.
## The plot wrapper promised the plotly API and delivered six methods
`ControlChartFigure`'s docstring said "maintaining full access to Plotly's
API". In fact `write_html`, `write_image`, `update_traces`, `add_hline` — the
things every plotly user reaches for first — raised AttributeError, and the
`.figure` escape hatch appeared nowhere in that error.
Now `__getattr__` delegates to the wrapped figure, with `__dir__` unioned so
completion finds both surfaces. Dunder names are refused rather than delegated:
letting `__deepcopy__`/`__reduce__` resolve to bound methods of the *figure*
makes copy, pickle and IPython display behave erratically, and `_fig` itself is
refused so an unpickle that runs `__getattr__` before the instance dict exists
cannot recurse forever. `write_html`/`write_image` are defined explicitly (not
left to fall through) so the latter keeps the error translation below.
## "Install kaleido" when kaleido was installed
`save_image` classified failures by substring-matching "kaleido" in the message,
so kaleido-present-but-no-browser — a completely different fix — was reported as
"Image export requires kaleido". People were told to reinstall a package they
already had.
`_translate_image_error` now separates the three: missing package (ImportError,
naming the extra), missing browser (RuntimeError, naming `plotly_get_chrome`),
and everything else (re-raised untouched, because not every failure here is
about the export backend).
## to_excel wrote a file it never mentioned
`export_html` defaulted to True, so `to_excel('analysis.xlsx')` also emitted a
standalone plotly document — multi-megabyte, unmentioned by the call, and
unreported by the return (there wasn't one). The workbook then carried an
"INTERACTIVE CHARTS" note promising those files.
`export_html` now defaults to False, `export()`/`to_excel()` return the list of
every path written, and the sheet note appears only when HTML was actually
written, naming the real filenames. The HTML step runs before the workbook is
written, since the sheet has to name files that exist. Per-chart image failure
also stops claiming to be a kaleido problem when it isn't, and a missing
kaleido short-circuits the remaining charts instead of logging N identical
warnings.
## Chart names an analyst actually types
`'xmr'` got a bespoke redirect; `'IMR'`, `'I-MR'`, `'Xbar-S'` — the Minitab
spellings this project's own README teaches as equivalent — got a bare "Unknown
chart". Now they all redirect with the spelling to use, and unrecognised names
get a difflib "Did you mean 'Xbar'?" at the two raise sites that know a chart
was meant.
Deliberately a redirect, not a silent alias: `XmR`/`IMR` name a chart *pair*, so
mapping either to one chart would guess which half was wanted. And deliberately
no did-you-mean inside `normalize_chart_name` — that function must pass unknown
names through, because stratum names flow through it and it cannot tell a typo
from a stratum.
`'xmr'` also raised a raw `ValueError` while its siblings raised
`ValidationError`; both are `ValidationError` now (which subclasses ValueError,
so existing handlers are unaffected).
## mypy was not running at all
`python_version = "3.9"` is rejected outright by current mypy, so type-checking
had been silently dead — `py.typed` was shipping annotations nothing verified.
3.10 does not fix it either: numpy's bundled stubs use 3.12 `type` statements
and mypy aborts on `numpy/__init__.pyi` before reaching our code. Set to 3.12
with the tradeoff written down — this is the stub-parsing version, not the
support floor, and ruff's `py310` target plus the 3.10 CI leg cover what it
stops catching.
With it running: 16 pre-existing errors in analysis.py (the TypedDict work
tracked in #47), and one I had just introduced — `_export_html_charts` still
annotated `-> None` while returning a list. Caught on the first run, which is
the argument for the whole exercise. Three more modules join the strict
per-module list (types, process_behavior, plotting.control_chart).
## Ruff
The `target-version` bump to py310 in the previous commit activated rules that
had never applied, leaving `ruff check .` — a hard CI gate — with 41 errors.
Fixed: PEP 604 annotations, sorted imports, yoda conditions, and `strict=False`
on every bare `zip()` (explicitly the behaviour-preserving choice; `strict=True`
would turn silent truncation into a runtime error, which is a decision for the
call sites, not a lint sweep).
## Docs
- excel-export: openpyxl and kaleido were described as automatically installed.
They are extras — that is the whole point of the dependency boundary.
- Five pages linked into tutorials that the previous commit pulled from the TOC.
## Tests
- `pytest tests/ -m "not slow"` — 2170 passed, 6 skipped.
- All six allowlisted notebooks execute clean.
- `ruff check .` clean; `mypy processbehavior` at its pre-existing baseline.
- New `tests/test_control_chart_figure.py` (delegation, dunder guard, copy
safety, the three error-translation branches) and `TestExportWritesWhatItSays`
(which asserts on-disk files exactly equal the reported list).
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
AnalysisDataSet,AnalysisSpecification, andStudyscipy.special.loggammawith stdlibmath.lgamma(~30MB saved)plot_residuals()diagnostic plotsTest plan
ruff check .passes with no errorsmypy processbehaviorpasses with no issues🤖 Generated with Claude Code