Conversation
qiskit-bot
added
the
Community PR
jakelishman
left a comment
jakelishman
left a comment
There was a problem hiding this comment.
I believe this to have been made by an unsupervised LLM, but I am prepared to accept something like it, with modifications, provided a human comes in the loop to respond and to claim ownership of all the changes.
Please re-instate and complete the correct LLM disclosure from the PR template that was deleted.
I will not continue if this continues to be an unsupervised LLM.
| """Return a string representation of ``inpt``, using simple ``pi`` forms when possible. | ||
|
|
||
| For numeric inputs, ``pi_check`` formats the real and imaginary parts independently. Within | ||
| ``eps``, it prefers integer multiples of pi, powers of pi from ``pi**2`` through ``pi**4`` | ||
| (except for ``output="qasm"``), and reduced fractions of the form ``n*pi/d`` or | ||
| ``n/(d*pi)`` with numerators and denominators up to :data:`MAX_FRAC`. If none of those | ||
| patterns match, the value falls back to ordinary numeric formatting; ``ndigits`` only affects | ||
| this fallback formatting. | ||
|
|
||
| String inputs are returned unchanged. For :class:`~qiskit.circuit.ParameterExpression` | ||
| inputs, numeric coefficients are rewritten using the same rules while preserving the symbolic | ||
| structure of the expression's string form. | ||
|
|
There was a problem hiding this comment.
This is unnecessarily verbose. It is not necessary or desirable for documentation to spell out the precise behaviour of an open-source function, especially for a non-public function (like this).
This is a private function and the docstring should convey the high-level intent in as few words as possible - the high-level intent here is that the function attempts to produce a representation of simple fractions of pi, if any contained floating-point values are sufficiently close.
There was a problem hiding this comment.
@jakelishman Hi there, I am a human(my linkdin profile link is in the bio), I have used Codex for solving the issue, and I am open to any kind of suggestions possible. Please let me know what kinds of suggestions you want me to make.
| output (str): Output style. Supported values are ``"text"`` (default), ``"latex"``, | ||
| ``"mpl"``, and ``"qasm"``. |
There was a problem hiding this comment.
The "supported values" line can be removed if the function is instead type-annotated by Literal["text", "latext", "mpl", "qasm"].
| def test_invalid_output_raises(self): | ||
| """Test invalid output styles raise.""" | ||
| with self.assertRaisesRegex( | ||
| QiskitError, "pi_check parameter output should be text, latex, mpl, or qasm." |
There was a problem hiding this comment.
It's not necessary to include the entire output message in the regex - that makes it harder to expand the options in the future. It's better to include a snippet long enough to uniquely (and obviously for the test reader) identify the message, but small enough it is likely to resist sensible changes in the future.
3 participants
Summary
pi_checkfor numeric, string, andParameterExpressioninputsoutputandndigitsaffect the rendered stringTesting
source .venv-codex/bin/activate && python -m unittest test.python.circuit.test_toolsfixes #10391
AI tool used: OpenAI Codex (GPT-5)