-
Notifications
You must be signed in to change notification settings - Fork 0
docs: add docstrings and prepare for doc generation #20
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
WalkthroughThis update introduces comprehensive docstrings to all core modules and classes in the codebase, enhancing internal documentation without altering any functionality. Additionally, it adds a complete MkDocs-based documentation system, including configuration, reference stubs, and custom CSS for improved documentation styling. No logic or control flow changes are present. Changes
Poem
📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (1)
✅ Files skipped from review due to trivial changes (1)
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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 3
🔭 Outside diff range comments (4)
api_lib/objects/request.py (2)
18-25
:default=None
makes it impossible to set an explicitNone
default
APIfield
treatsdefault is None
as “no default supplied”, which prevents callers from declaring a field whose legitimate default value isNone
. Use a sentinel (e.g.dataclasses.MISSING
) to differentiate “parameter omitted” from “parameter explicitly set toNone
”.-from typing import Any, Optional +from typing import Any, Optional +from dataclasses import MISSING ... -def APIfield(api_key: Optional[str] = None, default: Optional[Any] = None, **kwargs): +_UNSET = MISSING + +def APIfield(api_key: Optional[str] = None, default: Any = _UNSET, **kwargs): @@ - if default is None: + if default is _UNSET: return field(metadata=metadata, **kwargs) else: return field(default=default, metadata=metadata, **kwargs)
62-70
:as_array
filters out falsey values, not justNone
The docstring promises “non-None values”, but the comprehension drops any falsey value (0
,False
,""
). Compare withas_query_parameters
, which does the correctis not None
check.-return [f.metadata.get("api_key", f.name) for f in fields(self) if getattr(self, f.name)] +return [ + f.metadata.get("api_key", f.name) + for f in fields(self) + if getattr(self, f.name) is not None +]api_lib/api_lib.py (2)
65-73
:logger.debug
call uses positional arg instead ofextra=
Passing a dict as a second positional argument is ignored by the logger. Use keywordextra
or embed the fields in the format string.-logger.debug( - "Hitting API", - { - "host": self.host, - "method": method.value, - "path": path, - "data": getattr(data, "as_dict", {}), - }, -) +logger.debug( + "Hitting API %s %s", + method.value, + path, + extra={ + "host": self.host, + "data": getattr(data, "as_dict", {}), + }, +)
74-79
: Auth header is silently dropped
aiohttp.ClientSession
is created with the token header, but the per-request call passesheaders=self._headers
, overwriting session headers and losing the authorization. Merge the two dictionaries.- async with getattr(s, method.value)( + headers = {**getattr(self.token, "header", {}), **self._headers} + async with getattr(s, method.value)( url=f"{self.host}{self.prefix if use_api_prefix else ''}{path}", json={} if data is None else data.as_dict, - headers=self._headers, + headers=headers, ) as res:
🧹 Nitpick comments (12)
docs/css/mkdocstrings.css (1)
24-27
: Verify custom property fallback for accent color
var(--md-accent-fg-color)
is theme-specific; in consumer projects using a different palette the variable may be undefined, resulting in transparent icons on hover. Consider adding a fallback:background-color: var(--md-accent-fg-color, currentColor);docs/reference/headers/header.md (1)
1-4
: Minor: trailing blank line can be droppedWhile harmless, the extra blank line after the directive slightly inflates diff noise in similar stubs. Feel free to remove for consistency.
pyproject.toml (1)
28-30
: Relax or pin the new dev-dependency specifiers for safer CI reproducibility
mkdocs>=1.6.1
andmkdocstrings[python]>=0.28.0
leave the minor/major range open.
For tooling that is executed in CI and locally, unexpected breaking changes may slip in when a new minor/major is released.-"mkdocs>=1.6.1", -"mkdocstrings[python]>=0.28.0", +"mkdocs~=1.6.1", # 1.6.\* only +"mkdocstrings[python]~=0.28", # 0.28.\* only(Use the compatible release operator
~=
or freeze to an exact version and rely on dependabot/renovate for upgrades.)docs/index.md (1)
1-6
: Add an explicit link to the reference sectionA quick link improves UX and avoids the extra click through the side-nav.
- - Browse the API Reference for details on all modules and classes. + - [API Reference](reference/) – Detailed docs for all modules and classes.docs/reference/method.md (1)
1-4
: Minor MkDocStrings directive polishThe leading back-ticks inside the heading render verbatim. MkDocs already displays the file title; duplicating the code-styled module name is verbose. Consider:
-# `api_lib.method` - -::: api_lib.method +# api_lib.method + +::: api_lib.methoddocs/reference/headers/accept.md (1)
1-4
: Consistent heading styleFor consistency with other reference stubs (and to improve searchability) drop the back-ticks and stick to plain text, e.g.
# api_lib.headers.accept
.docs/reference/headers/authorization.md (1)
1-4
: Terminate the file with a newlineMkDocs-Markdown processors occasionally emit a warning when the last line of a document is not newline-terminated.
Appending a single\n
at EOF avoids such warnings and keeps the repo-wide formatting consistent.api_lib/headers/header.py (1)
30-37
:self.value
can remainNone
– guard against invalid headersWhen neither
value
nor a populatedenv_var
is provided,self.value
becomesNone
.
__repr__
/header
will then emit e.g.{'Authorization': 'Bearer None'}
which is usually unintended.Consider enforcing a non-nullable value:
else: raise KeyError(f"Environment variable '{env_var}' is not set or empty.") - self.value = value + if value is None: + raise ValueError("Either 'value' or a non-empty 'env_var' must be provided.") + self.value = valueapi_lib/objects/response.py (1)
10-26
: Docstring: clarify thatdefault
is metadata not the dataclass defaultThe implementation stores
default
inmetadata
rather than passing it tofield(default=…)
.
Readers might assume it behaves like the standarddataclasses.field(default=…)
.Add a brief note to avoid confusion.
api_lib/objects/request.py (1)
45-52
: Header keys are not lower-cased despite the docstring
as_header_string
lower-cases only the value part; the key remains untouched, which contradicts “lowercase header string”. Either change the implementation or re-phrase the docstring.-return "&".join([f"{k}={str(v).lower()}" for k, v in self.as_dict.items()]) +return "&".join([f"{k.lower()}={str(v).lower()}" for k, v in self.as_dict.items()])api_lib/headers/accept.py (1)
12-18
: Clarify behaviour whenaccept_type
is omitted
__init__
leavesself.value
empty whenaccept_type
isNone
, producing anAccept
header with no value. Consider raising ifaccept_type
is not supplied, or document that an empty value is intentional.api_lib/api_lib.py (1)
255-265
: Unnecessaryelse
afterreturn
(R1705)
Theelse
block is superfluous; the precedingreturn
already exits the function.- else: - return is_ok + return is_ok
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
uv.lock
is excluded by!**/*.lock
📒 Files selected for processing (18)
api_lib/api_lib.py
(7 hunks)api_lib/headers/accept.py
(1 hunks)api_lib/headers/authorization.py
(1 hunks)api_lib/headers/header.py
(2 hunks)api_lib/method.py
(1 hunks)api_lib/objects/request.py
(3 hunks)api_lib/objects/response.py
(5 hunks)docs/css/mkdocstrings.css
(1 hunks)docs/index.md
(1 hunks)docs/reference/api_lib.md
(1 hunks)docs/reference/headers/accept.md
(1 hunks)docs/reference/headers/authorization.md
(1 hunks)docs/reference/headers/header.md
(1 hunks)docs/reference/method.md
(1 hunks)docs/reference/objects/request.md
(1 hunks)docs/reference/objects/response.md
(1 hunks)mkdocs.yml
(1 hunks)pyproject.toml
(2 hunks)
🧰 Additional context used
🧬 Code Graph Analysis (2)
api_lib/objects/response.py (1)
api_lib/objects/request.py (2)
APIfield
(7-25)as_dict
(32-42)
api_lib/objects/request.py (1)
api_lib/objects/response.py (2)
APIfield
(10-26)as_dict
(62-68)
🪛 Pylint (3.3.7)
api_lib/headers/authorization.py
[refactor] 10-10: Too few public methods (1/2)
(R0903)
[refactor] 16-16: Too few public methods (1/2)
(R0903)
[refactor] 22-22: Too few public methods (1/2)
(R0903)
api_lib/headers/accept.py
[refactor] 22-22: Too few public methods (1/2)
(R0903)
[refactor] 28-28: Too few public methods (1/2)
(R0903)
[refactor] 34-34: Too few public methods (1/2)
(R0903)
[refactor] 40-40: Too few public methods (1/2)
(R0903)
[refactor] 46-46: Too few public methods (1/2)
(R0903)
[refactor] 52-52: Too few public methods (1/2)
(R0903)
[refactor] 58-58: Too few public methods (1/2)
(R0903)
[refactor] 64-64: Too few public methods (1/2)
(R0903)
api_lib/api_lib.py
[refactor] 266-271: Unnecessary "else" after "return", remove the "else" and de-indent the code inside it
(R1705)
🪛 YAMLlint (1.37.1)
mkdocs.yml
[warning] 38-38: wrong indentation: expected 2 but found 0
(indentation)
[error] 38-38: no new line character at the end of file
(new-line-at-end-of-file)
🔇 Additional comments (6)
docs/reference/api_lib.md (1)
1-4
: Looks good – consistent with other reference stubsNo issues spotted. 👍
docs/reference/objects/request.md (1)
1-4
: Reference stub OKStub is minimal yet sufficient; no action required.
docs/reference/objects/response.md (1)
1-4
: Reference stub OKMatches the pattern of other files. No concerns.
pyproject.toml (1)
65-65
: Confirmhatch-vcs
picks up tags as expectedSwitching
source = "vcs"
means your wheel version now entirely depends on git tags.
Double-check that:
- All tags follow the expected semantic-version pattern.
- CI pipelines always run in a clone that contains the tags (e.g.
git fetch --tags
).Otherwise the build may end up with
0.0.0
and break PyPI uploads.api_lib/method.py (1)
1-15
: Docstring looks good – one micro-clarificationIn an
Enum
, members are usually documented in the docstring’s Examples or list inside the body rather than an Attributes section (which tools may mis-interpret as dataclass fields).
No action required if your doc generator renders this correctly.api_lib/headers/authorization.py (1)
22-25
: ```shell
#!/bin/bashSearch for occurrences of the exact string "Apikey" in the repository
rg '"Apikey"' -n || true
</details> </blockquote></details> </details> <!-- This is an auto-generated comment by CodeRabbit for review status -->
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
Summary by CodeRabbit
Documentation
Chores