Skip to content

docs: fix formatting errors in ARModel docstrings - #442

Closed
alphaleporus wants to merge 1 commit into
mllam:mainfrom
alphaleporus:fix/ar-model-docstrings
Closed

docs: fix formatting errors in ARModel docstrings#442
alphaleporus wants to merge 1 commit into
mllam:mainfrom
alphaleporus:fix/ar-model-docstrings

Conversation

@alphaleporus

Copy link
Copy Markdown

Describe your changes

Summary of the changes:
This PR fixes strict reStructuredText (reST) formatting errors (specifically unexpected unindents and missing blank lines) in the docstrings of ARModel (common_step, plot_examples, and aggregate_and_plot_metrics).

Motivation and context:
As requested by @sadamov in PR #428, this PR isolates the docstring formatting fixes into a single, focused contribution. These formatting errors previously prevented Sphinx and other documentation generators from properly parsing the parameter and return blocks.

Dependencies:
None.

Issue Link

Relates to #61, #69 (Extracted from closed PR #428)

Type of change

  • 🐛 Bug fix (non-breaking change that fixes an issue)
  • ✨ New feature (non-breaking change that adds functionality)
  • 💥 Breaking change (fix or feature that would cause existing functionality to not work as expected)
  • 📖 Documentation (Addition or improvements to documentation)

Checklist before requesting a review

  • My branch is up-to-date with the target branch - if not update your fork with the changes from the target branch (use pull with --rebase option if possible).
  • I have performed a self-review of my code
  • For any new/modified functions/classes I have added docstrings that clearly describe its purpose, expected inputs and returned values
  • I have placed in-line comments to clarify the intent of any hard-to-understand passages of my code
  • I have updated the README to cover introduced code changes
  • I have added tests that prove my fix is effective or that my feature works
  • I have given the PR a name that clearly describes the change, written in imperative form (context).
  • I have requested a reviewer and an assignee (assignee is responsible for merging). This applies only if you have write access to the repo, otherwise feel free to tag a maintainer to add a reviewer and assignee.

Checklist for reviewers

Each PR comes with its own improvements and flaws. The reviewer should check the following:

  • the code is readable
  • the code is well tested
  • the code is documented (including return types and parameters)
  • the code is easy to maintain

Author checklist after completed review

  • I have added a line to the CHANGELOG describing this change, in a section
    reflecting type of change (add section where missing):
    • added: when you have added new functionality
    • changed: when default behaviour of the code has been changed
    • fixes: when your contribution fixes a bug
    • maintenance: when your contribution is relates to repo maintenance, e.g. CI/CD or documentation

Checklist for assignee

  • PR is up to date with the base branch
  • the tests pass
  • (if the PR is not just maintenance/bugfix) the PR is assigned to the next milestone. If it is not, propose it for a future milestone.
  • author has added an entry to the changelog (and designated the change as added, changed, fixed or maintenance)
  • Once the PR is ready to be merged, squash commits and merge the PR.

@Seai5

Seai5 commented Mar 19, 2026

Copy link
Copy Markdown

Hi @alphaleporus!

I just set up the repo locally (uv + editable + pre-commit) and ran a 1-epoch training to look around the code.
The ARModel docstrings look much cleaner now .

Two tiny suggestions if you want to make it even better before merge:

  1. In common_step, the :param: lines — maybe add a blank line after each parameter description? (Sphinx sometimes likes extra space there.)
    Example:
    :param x: Input tensor
    (with blank line here)
    :param y: ...

  2. Can we add one more Google-style section at the top of ARModel class docstring? Like:
    """Autoregressive model for neural LAM forecasting.

    Attributes:
    ...

    """

Happy to help test it locally with Sphinx if you add conf.py later.

Excited to contribute more docstring cleanups or Sphinx setup next!

Best,
Hrithik

@alphaleporus

Copy link
Copy Markdown
Author

Hi @Seai5, thanks for testing locally and the kind words!
Regarding the suggestions: I want to be careful about scope creep here. Per @sadamov's feedback on my previous PR, I was asked to keep this strictly limited to fixing the specific unexpected unindent formatting errors that were actively breaking the Sphinx build. Expanding the scope to reformat spacing or add new Attributes: sections to the class docstring is a great idea for a broader documentation overhaul, but I want to keep this PR as minimal as possible for the reviewers.
Happy to revisit if a maintainer prefers otherwise!

@sadamov

sadamov commented Mar 21, 2026

Copy link
Copy Markdown
Collaborator

@alphaleporus please direct this PR into #252 PR branch or add a review there with your suggestions (since they are minor)

@joeloskarsson

Copy link
Copy Markdown
Collaborator

Agreed, better to add to #252

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.

4 participants