Repository navigation
[CLEANUP] Post-0.5.0 tidy — dtype gap, dead code, CLI docs from parser, doc refresh - #105
Merged
Merged
Conversation
Contributor
There was a problem hiding this comment.
🟡 Changes recommended
Add regression coverage for the timedelta and warning paths and correct the noted contributor documentation wording.
3 open findings
What changed in this PR
Post-0.5.0 cleanup covering dtype preservation, report context removal, warning visibility, and documentation updates.
Changes:
- Preserve
timedelta64dtypes and warn on isopycnal rendering failures. - Remove obsolete
proc_machinecontext and related test/template code. - Generate CLI documentation from the parser and refresh project guidance.
| File | Reviewed changes |
|---|---|
tests/unit/test_report_golden.py |
Removed obsolete hostname mocking. |
README.md |
Clarified project scope and sibling tools. |
pyproject.toml |
Added the documentation dependency. |
oceanarray/utilities.py |
Added timedelta dtype passthrough; a direct regression test is requested. |
oceanarray/reports/templates/_macros.html |
Removed stale context documentation. |
oceanarray/reports/_stack.py |
Removed obsolete report context. |
oceanarray/reports/_mooring.py |
Removed hostname context generation. |
oceanarray/reports/_instrument.py |
Removed hostname context generation. |
oceanarray/reports/_grid.py |
Added failure warnings; a focused warning-path test is requested. |
oceanarray/cli.py |
Improved CLI help text. |
docs/source/conf.py |
Enabled sphinx-argparse. |
docs/source/cli_reference.rst |
Generated command options from the parser. |
CONTRIBUTING.md |
Refreshed architecture, scope, workflow, and testing guidance; two wording corrections are requested. |
🧠 Review effort: Lite
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.


Summary
Low-risk cleanup with no behaviour change to any committed output or fixture: a timedelta dtype-passthrough fix in
find_best_dtype, removal of the deadproc_machinereport-context key, a warning in place of a silent exception swallow in the grid report, the CLI reference now generated from the argument parser (viasphinx-argparse) instead of hand-maintained flag tables, and a CONTRIBUTING/README refresh. The CFcurrent_directionstandard_name rename initially scoped here was pulled out to ship with 0.6.0 — it is an output-attribute change.What's changed
find_best_dtypenow passes through timedelta64 (dtype.kind == "m") alongside datetime64, the same as string/object dtypes. One-character gap;ctdcast'swriters/dtypes.pycopy has the same gap._isopycnal_ts_panelno longer swallows exceptions silently — it warns with the caught exception before returningNone, so a realisopycnal_datasetfailure is visible instead of rendering as an "unavailable" stub. Render output is unchanged (still a stub on failure).proc_machinereport-context key: it was computed in every generator but shown nowhere since the footer dropped the host. Dropped from_mooring,_instrument,_stack,_grid, the two now-unusedsocketimports, the stale_macros.htmlcomment, and the now-pointlesssocket.gethostnamemock in the golden test.report/→reports/, addedwriters/andinspect.py);ruff formatin the workflow step; canonical-units convention; the real report plot-style mechanism; real-fixture and golden-test rules; and a Scope section (fixed output contract vs. method choices open to alternatives). One-sentence scope added to the README (naming onlycaldipandctdcastas the adjacent raw-processing siblings).cli_reference.rstnow uses.. argparse::blocks per subcommand (viasphinx-argparse, added to thedocsextra andconf.py), so the flag lists cannot drift frombuild_parser(). All prose, stage descriptions, "output created" sections and examples stay hand-written. Twohelp=strings enriched so nothing is lost (--pdfreading order;--arrayreworded to drop a bare*that RST read as emphasis). This corrected a stale--stagedefault ("1 2" → the real "1 2 3").Breaking changes
None. (The
current_directionCF standard_name rename, an output-attribute change, was deferred to the 0.6.0 cycle and is not in this PR.)Tests
793 unit tests pass (
tests/unit, excluding slow/local/seasenselib); 28 CLI tests pass; ruff clean; golden report tests green. The docs build was verified withsphinx-argparseinstalled —cli_referencerenders all eight subcommands with no warnings (only expected cross-doc:doc:links in isolation). No new test added —find_best_dtype's timedelta path and the isopycnal warn are covered by existing report/dtype tests; the proc_machine removal is a pure deletion of unrendered context.Notes
The
--helpoutput changes slightly: the generated CLI reference makes the parser'shelp=strings the documentation, so two were enriched (--pdfreading order,--arrayrewording). Thesphinx-argparsedependency is docs-only (in thedocs/devextras, not runtime).