docs: describe the returned object in Returns: for already-merged operations - #4148
Conversation
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files
|
ianna
left a comment
There was a problem hiding this comment.
@TaiSakuma - Looks great! Thanks for fixing it. It's good to get merged if you are done with it.
ikrommyd
left a comment
There was a problem hiding this comment.
The comments from #4130 are valid for this one too. I think that we had for "returns" here should just be moved right below it as normal text/description of the function right and the returns statement should be a sentence in the fashion of the "Args" descriptions right above.
|
@ikrommyd - I think that I understand what you have been saying. I just realized that the table in the PR description and the equivalent tables in the other PRs are wrong. The |
|
Here's the corrected table showing the full
🤖 Generated with Claude Code |
Proposed
|
…rations. Refs scikit-hep#3980. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…tion. Refs scikit-hep#3980. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
9f1b49e to
f28bb49
Compare
|
@ikrommyd - The proposed change in the above comment has been implemented. |
|
The documentation preview is ready to be viewed at http://preview.awkward-array.org.s3-website.us-east-1.amazonaws.com/PR4148 |
|
Good! One final style thing. Do you think it's best to have the longer description above the parameters like we do now (http://preview.awkward-array.org.s3-website.us-east-1.amazonaws.com/PR4148/reference/generated/ak.zip.html) or have it below the returns and above the examples? |
|
Yes. I think the space above the first heading ("Parameters") is the best place for a description that doesn't belong to any specific heading. I think that it is quite uncommon to have text without a heading between the Returns and Examples sections. If we want the text there, that should be part of the Returns section. |
|
Well I think we already have text in multiple places that is between the returns statement and the examples. Anything below the returns statement is just anything that can be in a docstring. When it comes to where the longer description should live, I also lean towards above the parameters unless it's super long. I just think that the user should be able to find the short description and the parameters/return arguments and types very quickly. |
|
@TaiSakuma I guess now that we have finalized the style, you can tell claude to update all similar PRs with this style. |
|
This PR has been merged. Thank you, @ianna and @ikrommyd, for the review. To recap:
|
Summary
Follow-up to #3980. The structural batch (#3986) added one-line summaries to those 13 operations, but 8 of them kept a
Returns:section that restates the operation instead of describing the returned object. @ikrommyd flagged this pattern while reviewing the open batch PRs (e.g. #4130), noting that a few already-merged cases need the same fix. This PR applies the object-style convention to those 8.Only the first sentence of each
Returns:is reworded; the explanatory text below it is unchanged.Returns
ak.ziparrays, combined into a single structure.ak.unziparray.ak.zip_no_broadcastarrays, combined into a single structure.ak.cartesianarrays.ak.argcartesianarrays, likeak.cartesianbut for use with__getitem__.ak.combinationsarraywith itself, restricted to combinations sampled without replacement.ak.argcombinationsarraywith itself, likeak.combinationsbut for use with__getitem__.ak.pad_nonelength.Notes for reviewers
ruff check,ruff format, andcodespellpass on the modified files.Refs #3980— no closing keyword, so the tracking issue stays open.AI disclosure
Drafted with Claude Code (Claude Opus 4.8) and reviewed manually, per the contribution agreement in #3980.
🤖 Generated with Claude Code