Skip to content

Pre-docs cleanup: fix the typo'd names and tighten the lint gate - #443

Open
kozlov721 wants to merge 19 commits into
mainfrom
chore/pre-docs-cleanup
Open

Pre-docs cleanup: fix the typo'd names and tighten the lint gate#443
kozlov721 wants to merge 19 commits into
mainfrom
chore/pre-docs-cleanup

Conversation

@kozlov721

@kozlov721 kozlov721 commented Sep 2, 2026

Copy link
Copy Markdown
Collaborator

Purpose

Prepare the repository for the Google docstyle migration and clean it
up in general. The PR mirrors the pre-docs cleanup in modelconverter
(luxonis/modelconverter#276) and the "Small breaking changes" PR in
luxonis-ml (luxonis/luxonis-ml#426). The docstyle migration follows
in a separate PR; it converts the docstrings that this PR makes
correct.

Specification

  • Add the typos pre-commit check with the same pin as the sibling
    repositories, and fix the 75+ typos it found.
  • Rename the typo'd public names:
    • reparametrizereparameterize (base class, module file,
      GeneralReparameterizableBlock, methods).
    • AttentionRefinmentBlockAttentionRefinementBlock.
    • mask_coeficientsmask_coefficients (head output key, loss
      parameter, tests — renamed together, so the wiring holds).
    • PPLCNetV3 detecion_blocksdetection_blocks.
  • Expand the ruff configuration to the modelconverter rule set:
    ASYNC, EXE, G, PGH, SLOT, full PERF, formatted docstring
    code examples, and a ban on __future__ imports. Fix the new
    findings and bump the ruff hook to match the ~=0.16 dev pin.
  • Fix all 49 pydoctor "documented parameter does not exist" warnings.
  • Correct the stale paths and examples in the README.
  • Exclude the stale build directory from pyright.
  • Anchor the head-branch regexes in .github/labeler.yaml, so a
    branch such as ci/release-automation no longer gets the release
    label and no longer disappears from the release notes.
  • Bump the prettier hook from v3.8.3 to v3.9.6.
  • Format the Python code blocks in README.md and examples/README.md.
  • Fix the "wit ha single key" typo in a node warning. The typos hook
    does not find it, because both words are correct.
  • Stop the variant metaclass from calling __init__ inside its
    except NotImplementedError handler. A node without variants often
    builds itself from a remote checkpoint. A network failure there
    chained onto the NotImplementedError, so the traceback opened with
    a variants error and hid the real cause.
  • Add unit tests for the bounding-box helpers, the tuner parameter
    parsing, and the metadata decode pass-through.

Out of scope, with their own follow-ups:

  • The remaining pydoctor warnings are format-level (@license fields,
    link targets). The docstyle migration rewrites those docstrings.
  • The typing debt (Any, cast, blanket type-ignores; PGH003 sits
    in the ignore list for now), like chore/type-gate-and-coverage in
    modelconverter.

Dependencies & Potential Impact

  • Breaking: the detecion_blocks attribute of PPLCNetV3 is a
    registered submodule, so its state-dict keys change. Old PPLCNetV3
    checkpoints do not load any more. The node has no remote weights
    URL, so only local checkpoints are affected.
  • Breaking: code that imports the renamed classes or calls the
    renamed methods must update the names. Configs are not affected; no
    registry name changes.
  • No dependency changes.

Deployment Plan

None / not applicable — the changes ship with the next release.

Testing & Validation

  • prek run --all-files: all hooks pass, including the new typos
    hook.
  • pydoctor --docformat=epytext: the 49 content warnings go to 0.
  • pyright --warnings: at exact parity with main (2 pre-existing
    aimet_torch import errors from the optional extra).
  • pytest -m unit: 547 passed. The 9 failures are the known expired
    gcloud-ADC failures in tests/unittests/test_losses, not code
    failures.

AI Usage

Assisted-by: Claude:claude-fable-5

Submitted code was reviewed by a human: NO

The author is taking the responsibility for the contribution: YES

🤖 Generated with Claude Code

https://claude.ai/code/session_01KXj7GZFCcdShtaTweRNH9j

Summary by CodeRabbit

  • Documentation

    • Corrected typos, parameter names, type descriptions, links, examples, and error messages across documentation and command references.
    • Updated model and configuration terminology for clearer guidance.
  • Refactor

    • Standardized reparameterization terminology across model components and export workflows.
    • Corrected public class, method, configuration, and result-key names for consistency.
  • Tests

    • Expanded coverage for tuning parameters, detection utilities, metadata decoding, and segmentation losses.
  • Chores

    • Improved linting and spelling checks, refreshed formatting rules, and refined automated branch labeling.

kozlov721 and others added 10 commits September 2, 2026 04:35
The check runs with the same pin as luxonis-ml and modelconverter.
The configuration allows `TOOD` (a paper acronym), `ScatterND` (an
ONNX operator), and skips `*.drawio` files, because `typos` mangles
their opaque element ids.

Most fixes change docstrings, comments, and messages. The rest rename
local variables (`padd`, `als`, `overlaps_thr_per_gt`), the
`check_n_workes_platform` validator, and the free-form model name in
`anomaly_detection_model.yaml`.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01KXj7GZFCcdShtaTweRNH9j
The rename covers the `Reparameterizable` base class, its module file,
`GeneralReparameterizableBlock`, the `reparameterize` methods, and the
related docstrings. Class and method names do not reach the state
dicts, so saved checkpoints still load.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01KXj7GZFCcdShtaTweRNH9j
The class name does not reach the state dicts, so saved checkpoints
still load.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01KXj7GZFCcdShtaTweRNH9j
The name is a wiring key: the head emits it in its output packet and
the loss receives it as a parameter of `forward`. The commit renames
the key, the parameter, and the tests together, so the wiring holds.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01KXj7GZFCcdShtaTweRNH9j
The attribute is a registered submodule, so the state-dict keys
change. Old PPLCNetV3 checkpoints do not load any more. The node has
no remote weights URL, so only local checkpoints are affected.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01KXj7GZFCcdShtaTweRNH9j
The configuration follows the modelconverter cleanup: the `ASYNC`,
`EXE`, `G`, `PGH`, `SLOT` groups, the full `PERF` group, formatted
docstring code examples, and a ban on `__future__` imports. The two
`__future__` users switch to string annotations. `PGH003` and
`PLR0917` go to the ignore list; the blanket type-ignores leave with
the type-gate follow-up.

The code fixes cover the new findings: two manual list builds turn
into `extend` calls, three unions move `None` to the end, the
duplicated `__all__` entries in `nodes/blocks` go away, one blanket
`noqa` gets its rule code, and the duplicated parameter formatting in
`LuxonisModelSummary` moves into a helper. The ruff hook moves to
v0.16.1 to match the `~=0.16` dev pin.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01KXj7GZFCcdShtaTweRNH9j
The gitignored `build` directory holds an old copy of the package that
setuptools leaves behind. Pyright read it and reported bogus
"X is not assignable to X" errors against the stale copy.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01KXj7GZFCcdShtaTweRNH9j
Pydoctor reported 49 "documented parameter does not exist" warnings.
Each doc entry now names the parameter that the signature has, or goes
away when the parameter no longer exists. The `recsubnet` docstring
also cited the DeiT paper title with the DRAEM link; the title now
matches the link.

The remaining pydoctor warnings are format-level: the `@license` and
`@copyright` fields, the missing link targets, and one `*args`
annotation the parser cannot read. The Google docstyle migration
rewrites those docstrings.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01KXj7GZFCcdShtaTweRNH9j
The custom-nodes link pointed at the dead `models/nodes/` path. The
custom-callback config example passed `lr` to a callback whose
parameter is `message`. The GCS protocol example spelled the bucket as
"buclet". The table of contents linked to a "key-features" anchor that
did not exist.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01KXj7GZFCcdShtaTweRNH9j
`actions/labeler` reads `head-branch` patterns as unanchored regular
expressions, so `"release/*"` matched any branch name that contains
"release". PR #437 (branch `ci/release-automation`) got the `release`
label and disappeared from the generated release notes, because
`.github/release.yaml` excludes that label. Every prefix pattern now
anchors with `^`.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01KXj7GZFCcdShtaTweRNH9j
@coderabbitai

coderabbitai Bot commented Sep 2, 2026

Copy link
Copy Markdown
Contributor

Review Change StackReview Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Advanced

Run ID: 6e9e8d22-65dc-4bac-bd02-58a4120a26f3

📥 Commits

Reviewing files that changed from the base of the PR and between 95e37de and 49bb738.

📒 Files selected for processing (5)
  • luxonis_train/core/utils/tune_utils.py
  • luxonis_train/utils/boundingbox.py
  • luxonis_train/variants.py
  • tests/unittests/test_utils/test_boxutils.py
  • tests/unittests/test_variants.py

Included review availability: Your plan provides up to 1 included review per hour; 0 remain after this review.


📝 Walkthrough

Walkthrough

The pull request standardizes spelling, naming, documentation, lint tooling, branch-label patterns, and test coverage. It also centralizes parameter-count formatting and renames the reparameterization API.

Changes

Repository consistency and naming cleanup

Layer / File(s) Summary
Tooling and documentation updates
.github/labeler.yaml, .pre-commit-config.yaml, pyproject.toml, README.md, examples/README.md
Branch patterns, pre-commit hooks, Ruff and Pyright settings, Typos settings, links, and examples are updated.
Public names and model references
luxonis_train/nodes/blocks/*, luxonis_train/nodes/base_node.py, luxonis_train/lightning/luxonis_lightning.py, luxonis_train/nodes/backbones/*, luxonis_train/nodes/necks/*
Reparameterization and attention block names are corrected. Imports, exports, calls, backbone references, and segmentation result keys use the corrected names.
Model and utility implementation updates
luxonis_train/callbacks/luxonis_model_summary.py, luxonis_train/core/utils/tune_utils.py, luxonis_train/utils/boundingbox.py, luxonis_train/variants.py
Parameter-count formatting is centralized. Optuna sampling uses suggest_float. Non-maximum suppression uses index-based filtering. Variant error handling is restructured.
Documentation and validation
luxonis_train/**, tests/unittests/*
Docstrings, comments, annotations, local names, error messages, and tests are aligned with existing interfaces and behavior.

Estimated code review effort: 3 (Moderate) | ~25 minutes

Merge Risk: 🔵 Low · up to 49bb7

The update modernizes tuning APIs and adds coverage, but the new tests still exercise dependency APIs with a documented removal path. This is a bounded compatibility concern requiring owner awareness before merge.

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 53.28% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 122 functions across 56 files. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly summarizes the main cleanup work, including typo and public-name corrections and stricter lint checks.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
  • Fix all pre-merge checks with AI
✨ Finishing Touches 💡 1
📝 Generate docstrings 💡
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch chore/pre-docs-cleanup

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@github-actions github-actions Bot added documentation Improvements or additions to documentation DevOps Changes related to DevOps CLI Changes affecting the CLI labels Sep 2, 2026
@codecov

codecov Bot commented Sep 2, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 94.79%. Comparing base (795f485) to head (49bb738).
✅ All tests successful. No failed tests found.

Additional details and impacted files
@@            Coverage Diff             @@
##             main     #443      +/-   ##
==========================================
+ Coverage   94.61%   94.79%   +0.18%     
==========================================
  Files         290      291       +1     
  Lines       15868    15941      +73     
==========================================
+ Hits        15013    15112      +99     
+ Misses        855      829      -26     
Flag Coverage Δ
pytest 94.79% <100.00%> (+0.18%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

@kozlov721
kozlov721 marked this pull request as ready for review September 7, 2026 22:59
@kozlov721
kozlov721 requested a review from a team as a code owner September 7, 2026 22:59
@kozlov721
kozlov721 requested review from klemen1999 and removed request for a team September 7, 2026 22:59
kozlov721 and others added 4 commits September 8, 2026 08:45
The hook stayed at v3.8.3 while the other hooks moved forward.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01C9WMoUvjBxLnqYrGJdNZrg
The examples used single quotes, loose spacing around the class
definitions, and lines above the limit. They now match the style that
ruff applies to the source.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01C9WMoUvjBxLnqYrGJdNZrg
The warning said "wit ha single key". The `typos` hook does not find
the fault, because both words are correct.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01C9WMoUvjBxLnqYrGJdNZrg
The three helpers had no unit tests. The bounding-box tests cover the
threshold guards, the single-class objectness copy, the class filter,
the box-format conversion, and the unknown reduction type. The tuner
tests cover the parameter parsing and its eight error paths. Two tests
pin the pass-through of `decode_text_metadata_labels` for a string
array and for a non-text type.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01C9WMoUvjBxLnqYrGJdNZrg
coderabbitai[bot]

This comment was marked as resolved.

kozlov721 and others added 2 commits September 8, 2026 09:24
A node without variants calls `get_variants`, which raises
`NotImplementedError`. The metaclass caught it and then called
`__init__` inside the handler. Such a node often builds itself from a
remote checkpoint, so a network failure in `__init__` chained onto the
`NotImplementedError`.

The traceback then opened with a variants error, and the real cause sat
below a "During handling of the above exception" line. A CI run showed
this: a gateway timeout from the torch hub surfaced as
`NotImplementedError` followed by `KeyError: 'Authorization'`.

The fallback now runs after the handler, so `__init__` raises on its
own.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01C9WMoUvjBxLnqYrGJdNZrg
kozlov721 and others added 2 commits September 8, 2026 09:56
`get_trial_params` called `suggest_loguniform` and `suggest_uniform`.
Optuna deprecated both in v3.0.0 and removes them in v6.0.0. The project
pins `optuna~=4.6`, so both calls raise a `FutureWarning` today.

The `_loguniform` and `_uniform` config suffixes stay the same. No user
config needs a change.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_013w7gCMnb22CH2DoAFqCePo
`non_max_suppression` built a boolean mask over the boxes above the
threshold, then concatenated it with the per-pair class indices. A box
above the threshold on two classes gave one masked row but two class
indices, so `torch.cat` raised:

    RuntimeError: Sizes of tensors must match except in dimension 1.
    Expected size 1 but got size 2 for tensor number 1 in the list.

That is the only case `multi_label=True` exists for, so the flag failed
on every real input.

An index tensor now replaces the mask. It repeats a box once for each of
its classes. The `has_additional` branch uses the same index, so the
trailing columns repeat with the box.

No caller in the project passes `multi_label=True`. The single-label
branch keeps the same rows in the same order, so its output does not
change.

The old test asserted a result that `multi_label=False` also gives, and
its comment named a function that does not exist. Two tests replace it.
The first asserts three detections from two boxes. The second covers the
trailing columns.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_013w7gCMnb22CH2DoAFqCePo
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

CLI Changes affecting the CLI DevOps Changes related to DevOps documentation Improvements or additions to documentation

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant