Skip to content

docs: fix typos, spelling and docstring formatting in vectorisers#195

Open
jamie-ons wants to merge 5 commits into
190-documentation-tidyup-for-v110from
vectorisers-documentation-tidyup-for-v110
Open

docs: fix typos, spelling and docstring formatting in vectorisers#195
jamie-ons wants to merge 5 commits into
190-documentation-tidyup-for-v110from
vectorisers-documentation-tidyup-for-v110

Conversation

@jamie-ons

Copy link
Copy Markdown

✨ Summary

Tidy up docstrings across the vectorisers module

📜 Changes Introduced

  • fixed typos
  • converted english us -> english uk with the exception of arguments such as tokenizer.
  • Capatalised Ollama in non code places
  • Updated __init__.py module overview to be consistent with base.py as they were near identical apart from a few words.
  • Removed some attributes as they weren't attributes in the code and were documented in the methods arguments

✅ Checklist

  • Code passes linting with Ruff
  • Security checks pass using Bandit
  • API and Unit tests are written and pass using pytest
  • Terraform files (if applicable) follow best practices and have been validated (terraform fmt & terraform validate)
  • DocStrings follow Google-style and are added as per Pylint recommendations
  • Documentation has been updated if needed

🔍 How to Test

@jamie-ons jamie-ons marked this pull request as ready for review June 24, 2026 10:28
@jamie-ons jamie-ons requested a review from a team as a code owner June 24, 2026 10:28

@rileyok-ons rileyok-ons 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.

Ran it over with some stricter doc linting, seems great, got a couple newlines you need after the docstring summary, https://docs.astral.sh/ruff/rules/missing-blank-line-after-summary/ checked and its part of the google python style guide https://google.github.io/styleguide/pyguide.html#38-comments-and-docstrings
A docstring should be organized as a summary line (one physical line not exceeding 80 characters) terminated by a period, question mark, or exclamation point. We currently have an ignore for this rule in our ruff config when actually we shouldn't, I'm going to remove it in my changes

Comment thread src/classifai/vectorisers/base.py
Comment thread src/classifai/vectorisers/__init__.py
@github-actions github-actions Bot added the documentation Improvements or additions to documentation label Jun 24, 2026
@jamie-ons

jamie-ons commented Jun 24, 2026

Copy link
Copy Markdown
Author

Have added the new lines.

However have noticed that most one line summaries are over 80 characters. Should I rewrite all summaries to be <=80 characters. This would make it comply fully with the google python style guide but also seems quite strict?

We dont have any that are so long that they are unreadable but for example

Initialises the GcpVectoriser with the specified project ID, location, and model name.

This is 86 characters long and so wouldnt comply

@jamie-ons jamie-ons requested a review from rileyok-ons June 24, 2026 14:35
@rileyok-ons

Copy link
Copy Markdown
Contributor

Have added the new lines.

However have noticed that most one line summaries are over 80 characters. Should I rewrite all summaries to be <=80 characters. This would make it comply fully with the google python style guide but also seems quite strict?

We dont have any that are so long that they are unreadable but for example

Initialises the GcpVectoriser with the specified project ID, location, and model name.

This is 86 characters long and so wouldnt comply

Checked and current idea is we're not strict on the line length

@rileyok-ons

Copy link
Copy Markdown
Contributor

As an additional thing can you add types to the ABC vectorizser docstring

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

documentation Improvements or additions to documentation

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants