CI-1802: Expand and correct the SDK docstrings - #223
Open
sminot wants to merge 7 commits into
Open
Conversation
The `cirro` and `cirro.sdk` packages had no module docstrings, so the pdoc landing page and `help(cirro)` -- the first place a reader lands -- said nothing about how to authenticate or how the classes relate. `DataPortal()` falls through to a device-code login that blocks on a browser flow, which hangs in any non-interactive context. Neither the constructor nor `DataPortalLogin` mentioned this or pointed at `ClientCredentialsAuth`. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Four docstring examples opened with `from cirro import DataPortal()`, which is a SyntaxError. Two `Returns:` blocks named an import statement rather than the type returned. `run_analysis` documented `params` only as "Analysis parameters", with no pointer to `get_parameter_spec`, which is the only way to learn what keys a process accepts. It also did not say that the call is asynchronous or how to follow the resulting dataset. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Several properties return a snapshot that never updates, which reads as a hang when polling a running analysis: `dataset.status` is fixed at construction, and `dataset.logs` / `dataset.tasks` are `cached_property`, so an empty log read before the job starts stays empty for the object's lifetime. Each now says to re-fetch the dataset. `dataset.logs`, `dataset.get_logs()` and `task.logs` are three different logs with near-identical names; each now says which one it is and points at the others. `list_files` and `samples` now state that they truncate silently, and `force_refresh` says what it discards. Also documents the members that had no docstring at all -- `file_count`, `total_size_bytes`, `total_size`, `developer_helper`, `absolute_path`, the two `get_by_id` overrides that only raise -- and the lookup helpers on `DataPortalAssets`, which are what make every `list_*` result more than a list. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
The corpus mixed three dialects -- Google sections, reStructuredText roles and literals, and Markdown fences -- sometimes within one docstring. `make_docs.py` sets `docformat="google"`, and raw `help()` output shows the RST verbatim, so RST roles and ``literals`` are converted to Markdown backticks throughout. `DataPortalDataset.read_files` / `read_file` deferred to their `DataPortal` equivalents for the glob and pattern rules, leaving the docstring on the object you actually hold nearly empty; both are now self-contained. Fills in the Args, Returns and Raises sections still missing on the file-reading mixin, file downloads, the developer helpers, and `CirroApi.__init__`. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
sminot
changed the title
The head node may be running Nextflow or Cromwell depending on the process executor, so `dataset.logs` no longer calls it Nextflow's output. `get_logs` reads the `WORKFLOW_LOGS` artifact for either. `get_trace` stays Nextflow-only, since a Cromwell analysis produces no trace artifact -- now stated, along with Cromwell as a reason the artifact can be missing. `status` no longer enumerates the `Status` values. The list had omitted `DELETE`, and hardcoding a generated enum into a docstring only invites drift; it now points at `cirro_api_client.v1.models.Status` and names the values an analysis actually moves through. Restore the RST literals in `_pattern_to_captures_regex`. It is private, so pdoc renders no member section for it and `help()` is never called on it -- the only way anyone reads that docstring is as source, where the doubled delimiters mark where a punctuation literal like `*` or `**` starts and ends. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Rendering the pages before and after the conversion produces byte-identical HTML: pdoc already turns :func:`pandas.read_csv` into `pandas.read_csv()` and ``DataFrame`` into `DataFrame` under docformat="google", so the conversion was reproducing by hand what pdoc does anyway. It also discarded semantic markup that a Sphinx build would resolve into real links, and left `cirro/services` inconsistent with `cirro/sdk`. The docstrings rewritten for content keep the local style: RST for inline literals, Markdown fences for code blocks, matching DataPortal.read_files. task.py is no longer touched -- its whole diff was this churn. Two cross-references drop the `~` prefix, which pdoc does not honour: it renders ":meth:`~cirro.sdk.portal.DataPortal.read_files`" with the tilde still in the text, on main as well. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
sminot
changed the title
# Conflicts: # cirro/sdk/file.py
|
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
1 participant
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.
Users may want to automate interaction with Cirro using AI assistants via the python SDK. This PR expands the documentation provided with that library to streamline the process. There were also some errors that were caught in the process.
No code changes.
Written by Claude, reviewed by @sminot