Skip to content

Fix API docstring math and ASCII rendering - #8259

Open
huangtina22 wants to merge 3 commits into
quantumlib:mainfrom
huangtina22:fix/docs-latex-uniform-superposition-8212
Open

Fix API docstring math and ASCII rendering#8259
huangtina22 wants to merge 3 commits into
quantumlib:mainfrom
huangtina22:fix/docs-latex-uniform-superposition-8212

Conversation

@huangtina22

@huangtina22 huangtina22 commented Aug 5, 2026

Copy link
Copy Markdown

Replace nonstandard \ket with |j\rangle so the UniformSuperpositionGate formula renders correctly, and indent the integrated_histogram ASCII sketch as a Markdown code block so it does not collapse to one line in the docs.

Fixes #8212
Fixes #8211

@huangtina22
huangtina22 requested a review from a team as a code owner August 5, 2026 20:10
@huangtina22
huangtina22 requested a review from maffoo August 5, 2026 20:10
@google-cla

google-cla Bot commented Aug 5, 2026

Copy link
Copy Markdown

Thanks for your pull request! It looks like this may be your first contribution to a Google open source project. Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA).

View this failed invocation of the CLA check for more information.

For the most up to date status, view the checks section at the bottom of the pull request.

@github-actions github-actions Bot added the size: S 10< lines changed <50 label Aug 5, 2026
@mhucka

mhucka commented Aug 5, 2026

Copy link
Copy Markdown
Contributor

@tinakim522 It's unclear whether this account is an AI bot or a real person. In any case, the PR contents are incorrect: the changes use rST syntax, but this project does not use rST for docstrings.

If you are a real person, please let us know here.

@huangtina22

huangtina22 commented Aug 5, 2026 via email

Copy link
Copy Markdown
Author

@huangtina22
huangtina22 force-pushed the fix/docs-latex-uniform-superposition-8212 branch from d017797 to 15d86bc Compare August 6, 2026 04:10
@huangtina22

Copy link
Copy Markdown
Author

Thanks for the feedback — you're right, Cirq uses Markdown-flavored Google-style docstrings, not rST.

I pushed an update that:

  • keeps $...$ / $$...$$ LaTeX and replaces \ket{j} with |j\rangle
  • formats the integrated_histogram ASCII sketch as an indented Markdown code block (no ::)

Also still need to complete the Google CLA on my side.

Replace nonstandard \ket with |j\rangle and display math for
UniformSuperpositionGate, and indent the integrated_histogram ASCII
sketch as a Markdown code block so it does not collapse in docs.

Fixes quantumlib#8212
Fixes quantumlib#8211
@huangtina22
huangtina22 force-pushed the fix/docs-latex-uniform-superposition-8212 branch from 15d86bc to 13cf009 Compare August 6, 2026 04:11
@mhucka mhucka self-assigned this Aug 6, 2026
@mhucka

mhucka commented Aug 6, 2026

Copy link
Copy Markdown
Contributor

Ah, good, a real person! Thanks for your reply and contribution. I'll do a review now.

@codecov

codecov Bot commented Aug 6, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 99.59%. Comparing base (013d479) to head (817f0d7).

Additional details and impacted files
@@           Coverage Diff           @@
##             main    #8259   +/-   ##
=======================================
  Coverage   99.59%   99.59%           
=======================================
  Files        1120     1120           
  Lines      102249   102249           
=======================================
  Hits       101836   101836           
  Misses        413      413           

☔ 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.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@mhucka mhucka left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

A very small change request; otherwise, this looks good.

r"""Creates a uniform superposition state on the states $[0, M)$.

The gate creates the state
$$\frac{1}{\sqrt{M}}\sum_{j=0}^{M-1}|j\rangle$$

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Math formulas inside $$ need to be put on separate lines to format reliably in the software that generates the docs.

Suggested change
$$\frac{1}{\sqrt{M}}\sum_{j=0}^{M-1}|j\rangle$$
$$
\frac{1}{\sqrt{M}}\sum_{j=0}^{M-1}|j\rangle
$$

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Also, please make sure to update the branch one more time after the edits.

image

@huangtina22

Copy link
Copy Markdown
Author

Thanks for the review — updated: display math is now on separate lines, and the branch is synced with main.

@mhucka mhucka left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Thank you again.

@pavoljuhas

Copy link
Copy Markdown
Collaborator

Thanks for the review — updated: display math is now on separate lines, and the branch is synced with main.

Please communicate in your own words without copy/pasting from an AI tool.

@huangtina22

Copy link
Copy Markdown
Author

@pavoljuhas. Here i am talking in my words. Any further issues found?

@pavoljuhas

pavoljuhas commented Aug 6, 2026

Copy link
Copy Markdown
Collaborator

@pavoljuhas. Here i am talking in my words. Any further issues found?

Thank you. Please see the failing CLA check, we cannot accept PRs without it.

Also, please update the PR description so it is consistent with commit messages in this repo. Specifically, please remove the Summary heading and the Test plan can be moved to a separate comment. Otherwise LGTM.

@huangtina22

huangtina22 commented Aug 7, 2026

Copy link
Copy Markdown
Author

Test notes:

  • UniformSuperpositionGate docs: formula should render as math
  • integrated_histogram: ascii sketch should stay multi-line

@huangtina22

huangtina22 commented Aug 7, 2026

Copy link
Copy Markdown
Author

@pavoljuhas updated the description and moved the test notes to a separate comment. working on the CLA now.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

size: S 10< lines changed <50

Projects

None yet

Development

Successfully merging this pull request may close these issues.

LaTeX formatting bug in API docs for cirq.UniformSuperpositionGate API documentation rendering problem for integrated_histogram

3 participants