Skip to content

Add per-operation grouping review guideline for dplyr - #239

Merged
d-morrison merged 3 commits into
mainfrom
claude/per-operation-grouping-review-hjp2hz
Jun 27, 2026
Merged

Add per-operation grouping review guideline for dplyr#239
d-morrison merged 3 commits into
mainfrom
claude/per-operation-grouping-review-hjp2hz

Conversation

@d-morrison

Copy link
Copy Markdown
Collaborator

Summary

  • Adds shared/coding/per-operation-grouping.md with guidance to prefer dplyr's .by argument over persistent group_by()/ungroup() pairs when grouping is only needed for one operation.
  • References the new fragment from CLAUDE.md alongside the other coding guidelines.

Reference: https://dplyr.tidyverse.org/reference/dplyr_by.html
Example flagged in review: https://github.com/ucdavis/rampp/pull/125/changes/#diff-ef2f6f6745046859ccc877eb26de77f63a99edc851687c9fa053788458f7f731R265-R267


Generated by Claude Code

Flag persistent group_by()/ungroup() pairs in code review when the .by
argument would scope the grouping to a single operation instead.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01UZVwdp2UkrhAUDrh2cwdfC
@claude

This comment has been minimized.

Comment thread shared/coding/per-operation-grouping.md Outdated

Copy link
Copy Markdown
Collaborator Author

Driving this PR to clean — back off until done.


Generated by Claude Code

Simplify closing sentence: replace i.e. with "that is" and reduce
stacked subordinate clauses per the plain-prose guideline.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01UZVwdp2UkrhAUDrh2cwdfC

Copy link
Copy Markdown
Collaborator Author

ARD summary

Finding Disposition Action
Closing sentence stacks three subordinate clauses; i.e. instead of that is Address Simplified to two clauses, replaced i.e. with "that is" in shared/coding/per-operation-grouping.md

Generated by Claude Code

@claude

This comment has been minimized.

Comment thread shared/coding/per-operation-grouping.md Outdated
Split stacked subordinate clauses into two sentences per plain-prose guideline.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01UZVwdp2UkrhAUDrh2cwdfC

Copy link
Copy Markdown
Collaborator Author

ARD summary — round 2

Finding Disposition Action
Opening sentence stacks two subordinate clauses Address Split into two sentences; removed "R code that uses dplyr" → "dplyr code"

Generated by Claude Code

@claude

claude Bot commented Jun 27, 2026

Copy link
Copy Markdown
Contributor

Claude finished @d-morrison's task in 1m 29s —— View job


Review

  • Read changed files (current state)
  • Verify prior finding (opening sentence stacking) addressed
  • Scan for new issues (hallucinations, style, correctness)
  • Post verdict

Prior finding confirmed addressed. The round-2 inline finding — opening sentence at lines 3–5 stacking two subordinate clauses ("When reviewing or writing R code that uses dplyr" and "where the grouping is only needed for one operation") — is fully resolved. Lines 3–5 now match the suggested split exactly: one sentence stating the preference, a second applying the condition.

No new findings. Checked:

  • Prose style: Every sentence carries at most one subordinate clause. The closing sentence (lines 17–19) uses an em-dash + "that is" to clarify a single condition; that phrasing was the round-1 suggestion and stands.
  • Code examples: Both are correct dplyr 1.1.0+ syntax. summarise(..., .by = group_col) is real per-operation grouping syntax; the group_by() |> summarise() |> ungroup() pattern is accurate.
  • Reference URL: https://dplyr.tidyverse.org/reference/dplyr_by.html points to the real dplyr .by documentation.
  • CLAUDE.md wiring: The heading and @shared/coding/per-operation-grouping.md fragment reference follow the established pattern for other shared coding guidelines.
  • Hallucinations: None detected — all function names, argument names, and the reference URL check out against actual dplyr.

This PR is fully clean.

Copy link
Copy Markdown
Collaborator Author

Done — PR is free.


Generated by Claude Code

@d-morrison
d-morrison merged commit c64534a into main Jun 27, 2026
4 checks passed
@d-morrison
d-morrison deleted the claude/per-operation-grouping-review-hjp2hz branch June 27, 2026 02:36
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants