fix(docs): correct stale unit examples in array_creation notebook#128
Conversation
Two cells in docs/unit_operations/array_creation.ipynb used examples that violated saiunit's documented unit semantics and raised UnitMismatchError, breaking notebook execution: - `asarray([1, 2, 3], unit=second)`: a raw dimensionless list cannot be converted to seconds. `asarray` with an explicit `unit=` performs strict conversion (per its docstring and the notebook's own markdown), so the example now converts a same-dimension quantity across magnitudes (ms -> s), which is what the surrounding prose describes. - `logspace(0*s, 10*s, 5)`: `logspace` computes `base ** x`, which is intrinsically dimensionless and rejects unitful start/stop. The cell now demonstrates that constraint via try/except, matching the notebook's existing idiom for showing expected errors. All 31 source notebooks under docs/ now execute cleanly.
Reviewer's GuideUpdates the array_creation documentation notebook so that all examples respect saiunit’s unit semantics, fixing two stale code cells and re-executing the notebook to refresh outputs and execution metadata. File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
There was a problem hiding this comment.
Hey - I've found 1 issue, and left some high level feedback:
- Most of the diff is notebook execution metadata (timestamps, iopub events, Python version, etc.); consider stripping or normalizing these fields so future documentation changes have smaller, more focused diffs.
- The
triexample now captures a JAX dtype UserWarning in stderr; if this isn’t intentional instructional content, it would be better to adjust the example or environment so this warning doesn’t appear in the rendered docs.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- Most of the diff is notebook execution metadata (timestamps, iopub events, Python version, etc.); consider stripping or normalizing these fields so future documentation changes have smaller, more focused diffs.
- The `tri` example now captures a JAX dtype UserWarning in stderr; if this isn’t intentional instructional content, it would be better to adjust the example or environment so this warning doesn’t appear in the rendered docs.
## Individual Comments
### Comment 1
<location path="docs/unit_operations/array_creation.ipynb" line_range="13-22" />
<code_context>
{
"cell_type": "markdown",
"metadata": {},
</code_context>
<issue_to_address>
**suggestion:** Consider avoiding stderr warnings in documentation output for `u.math.tri`.
The `u.math.tri(3)` example currently emits a JAX `UserWarning` about truncating `float64` to `float32`, which will appear in the rendered docs and distract from the intended output. Consider either changing the example to avoid triggering the warning (e.g., by enabling `jax_enable_x64` or using a non-upcasted dtype) or explicitly suppressing this warning in the notebook cell so the visible output only shows the function result.
Suggested implementation:
```
"cell_type": "code",
"execution_count": 1,
"metadata": {
"ExecuteTime": {
```
```
"cell_type": "code",
"execution_count": 1,
"metadata": {},
"source": [
"import warnings\n",
"\n",
"with warnings.catch_warnings():\n",
" warnings.filterwarnings(\n",
" \"ignore\",\n",
" message=\".*truncating.*float64.*to float32.*\",\n",
" category=UserWarning,\n",
" )\n",
" u.math.tri(3)\n"
]
},
```
I assumed:
1. There is a code cell in `docs/unit_operations/array_creation.ipynb` whose sole (or final) source line is exactly `u.math.tri(3)`.
2. The JSON structure around that cell matches the pattern shown in the snippet.
If the actual notebook cell differs (e.g., multiple expressions or different execution counts/metadata), you'll need to adjust the `SEARCH` block to match the exact JSON for that cell. The key idea is to replace the bare `u.math.tri(3)` source with the `warnings`-wrapped version shown in the `REPLACE` block.
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
| { | ||
| "cell_type": "markdown", | ||
| "metadata": {}, | ||
| "source": "The functions listed below are designed to create `array` or `Quantity` with specific properties, such as filled with a certain value, identity matrices, or arrays with ones on the diagonal. These functions are part of the `saiunit.math` module and are tailored to handle both numerical `array` and `Quantity` with units." | ||
| "source": [ | ||
| "The functions listed below are designed to create `array` or `Quantity` with specific properties, such as filled with a certain value, identity matrices, or arrays with ones on the diagonal. These functions are part of the `saiunit.math` module and are tailored to handle both numerical `array` and `Quantity` with units." | ||
| ] | ||
| }, | ||
| { | ||
| "cell_type": "code", | ||
| "execution_count": 1, |
There was a problem hiding this comment.
suggestion: Consider avoiding stderr warnings in documentation output for u.math.tri.
The u.math.tri(3) example currently emits a JAX UserWarning about truncating float64 to float32, which will appear in the rendered docs and distract from the intended output. Consider either changing the example to avoid triggering the warning (e.g., by enabling jax_enable_x64 or using a non-upcasted dtype) or explicitly suppressing this warning in the notebook cell so the visible output only shows the function result.
Suggested implementation:
"cell_type": "code",
"execution_count": 1,
"metadata": {
"ExecuteTime": {
"cell_type": "code",
"execution_count": 1,
"metadata": {},
"source": [
"import warnings\n",
"\n",
"with warnings.catch_warnings():\n",
" warnings.filterwarnings(\n",
" \"ignore\",\n",
" message=\".*truncating.*float64.*to float32.*\",\n",
" category=UserWarning,\n",
" )\n",
" u.math.tri(3)\n"
]
},
I assumed:
- There is a code cell in
docs/unit_operations/array_creation.ipynbwhose sole (or final) source line is exactlyu.math.tri(3). - The JSON structure around that cell matches the pattern shown in the snippet.
If the actual notebook cell differs (e.g., multiple expressions or different execution counts/metadata), you'll need to adjust the SEARCH block to match the exact JSON for that cell. The key idea is to replace the bare u.math.tri(3) source with the warnings-wrapped version shown in the REPLACE block.
Summary
Executed all 31 source notebooks under
docs/(excludingdocs/_build/build artifacts). Two cells indocs/unit_operations/array_creation.ipynbraisedUnitMismatchErrorbecause they used examples inconsistent with saiunit's documented unit semantics. This fixes both so every notebook runs cleanly.Fixes
asarray([1, 2, 3], unit=second)→ a raw dimensionless list cannot be converted to seconds.asarraywith an explicitunit=performs strict conversion — consistent with its docstring, the audit comment in_fun_array_creation.py, and the notebook's own markdown. The cell now converts a same-dimension quantity across magnitudes (ms→s), matching the surrounding prose ("same dimension but different magnitude → converted to the provided unit").logspace(0*s, 10*s, 5)→logspacecomputesbase ** x, which is intrinsically dimensionless and rejects unitfulstart/stop(per its docstring). The cell now demonstrates that constraint viatry/except, matching the notebook's existing idiom for showing expected errors.No library code changed — both were stale documentation examples. Only
docs/unit_operations/array_creation.ipynbis modified (2 source cells + re-executed outputs).Verification
🤖 Generated with Claude Code
Summary by Sourcery
Update unit operations array creation documentation examples to align with saiunit’s unit semantics and ensure the notebook executes cleanly.
Bug Fixes:
Documentation: