Skip to content

DOC: tighten the comments that came with the sampler seed groups - #1154

Merged
Gui-FernandesBR merged 1 commit into
RocketPy-Team:developfrom
thc1006:doc/tighten-sampler-seed-comments
Aug 14, 2026
Merged

DOC: tighten the comments that came with the sampler seed groups#1154
Gui-FernandesBR merged 1 commit into
RocketPy-Team:developfrom
thc1006:doc/tighten-sampler-seed-comments

Conversation

@thc1006

@thc1006 thc1006 commented Aug 14, 2026

Copy link
Copy Markdown
Contributor

Pull request type

  • ReadMe, Docs and GitHub updates

Checklist

  • Tests for the changes have been added (if needed)
  • Docs have been reviewed and added / updated
  • Lint (ruff check / ruff format --check) has passed locally
  • All tests (pytest tests/unit) have passed locally

Current behavior

The comments I added in #1102 are longer than they need to be. Three of them
explain the same rule, and several narrate how the rule was arrived at instead
of stating it. seed_group in particular carries three paragraphs for a
one-line body, which is out of step with sample and reset_seed right below
it in the same file.

New behavior

Same rules, shorter. The contract a shared group has to honour now lives on
seed_group, and _reset_custom_samplers and _validate_custom_sampler point
at it rather than restating it. Inline comments state the constraint rather than
the investigation, and the #1103 reference is kept as a reference.

Nothing outside comments and docstrings changed. I checked that by parsing both
revisions, dropping every docstring, and comparing the resulting ASTs: identical
for both files.

Net effect is -21 lines.

Breaking change

  • No

Additional information

Local run against develop at cb6106a:

ruff check .            All checks passed
ruff format --check .   312 files already formatted
pylint rocketpy/ tests/ docs/    exit 0
pytest tests/unit       2036 passed, 12 skipped

One thing I left alone. seed_group's prose is down from 12 lines to 6, but its
body is return self, so it is still longer than the code it sits on. It is an
abstract property that users implement themselves, and the three things it says
(all members return the same generator, return the same object each call, one
group per model) are each a way to get a silently wrong seed. I would rather
keep those than hit a line count. Happy to cut it further if you disagree.

@thc1006
thc1006 requested a review from a team as a code owner August 14, 2026 07:52
@codecov

codecov Bot commented Aug 14, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 83.30%. Comparing base (e0ff281) to head (85d682d).
⚠️ Report is 58 commits behind head on develop.

Additional details and impacted files
@@             Coverage Diff             @@
##           develop    #1154      +/-   ##
===========================================
+ Coverage    82.18%   83.30%   +1.12%     
===========================================
  Files          122      130       +8     
  Lines        16355    17080     +725     
===========================================
+ Hits         13441    14229     +788     
+ Misses        2914     2851      -63     

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@thc1006
thc1006 force-pushed the doc/tighten-sampler-seed-comments branch from 5aefe54 to 5c9fb34 Compare August 14, 2026 08:12
The prose added in RocketPy-Team#1102 restates itself across three places and narrates how
each rule was arrived at. Keep the caller-visible rules, drop the retelling,
and let a shared group's contract live on seed_group rather than being
repeated at each call site. Comments only, no change to executable code.

Signed-off-by: thc1006 <84045975+thc1006@users.noreply.github.com>
@Gui-FernandesBR
Gui-FernandesBR force-pushed the doc/tighten-sampler-seed-comments branch from 5c9fb34 to 85d682d Compare August 14, 2026 09:50
@Gui-FernandesBR
Gui-FernandesBR merged commit 9d4726d into RocketPy-Team:develop Aug 14, 2026
8 checks passed
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