Skip to content

Conversation

@isaacdevlugt
Copy link
Contributor

Context: Came up in discussing #7606

Description of the Change: Reorders the docstrings of each optimizer: (1) brief description of optimizer (2) theory if needed (3) Parameters (4) examples if needed.

Benefits: consistency

Possible Drawbacks: 🤷

Related GitHub Issues:

@github-actions
Copy link
Contributor

Hello. You may have forgotten to update the changelog!
Please edit doc/releases/changelog-dev.md with:

  • A one-to-two sentence description of the change. You may include a small working example for new features.
  • A link back to this PR.
  • Your name (or GitHub username) in the contributors section.

@codecov
Copy link

codecov bot commented Jul 16, 2025

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 99.68%. Comparing base (bb029e1) to head (afcb3ad).
⚠️ Report is 414 commits behind head on master.

Additional details and impacted files
@@           Coverage Diff           @@
##           master    #7891   +/-   ##
=======================================
  Coverage   99.68%   99.68%           
=======================================
  Files         544      544           
  Lines       55413    55413           
=======================================
  Hits        55236    55236           
  Misses        177      177           

☔ View full report in Codecov by Sentry.
📢 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.

Copy link
Contributor

@SimoneGasperini SimoneGasperini left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good, thanks!

The only comment I have is that we might still be missing some default values for the parameters. Also, I noticed that sometimes we do e.g. stepsize=0.1 (float): ... while in other cases we do stepsize (float): ... (default: 0.1).

Don't know if it's worth to fix but it would be nice for consistency :)

@isaacdevlugt
Copy link
Contributor Author

@SimoneGasperini good catch! I updated the docstrings with your recommendation :)

Copy link
Contributor

@AntonNI8 AntonNI8 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks @isaacdevlugt!

@isaacdevlugt isaacdevlugt enabled auto-merge July 21, 2025 17:12
@isaacdevlugt isaacdevlugt added this pull request to the merge queue Jul 21, 2025
@github-merge-queue github-merge-queue bot removed this pull request from the merge queue due to a conflict with the base branch Jul 21, 2025
@isaacdevlugt isaacdevlugt enabled auto-merge July 21, 2025 18:57
@isaacdevlugt isaacdevlugt added this pull request to the merge queue Jul 21, 2025
Merged via the queue into master with commit 6894301 Jul 21, 2025
54 checks passed
@isaacdevlugt isaacdevlugt deleted the optimizer-docstring-order branch July 21, 2025 21:42
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.

6 participants