Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Support additional **kwargs with default arguments in Transformer API. #4890

Merged

Conversation

tanujkhattar
Copy link
Collaborator

@tanujkhattar tanujkhattar commented Jan 24, 2022

Using the trick originally introduced in #4871 to check for type annotations only in the signature of @cirq.transformer, we are now able to support methods which take additional **kwargs with a default value specified to be valid transformers.

This makes the writing transformers of the following form possible, with type annotations correctly working at both the decorator level and at the call site where the transformer is called using **kwargs.

    >>> @cirq.transformer
    >>> def convert_to_sqrt_iswap(
    >>>     circuit: cirq.AbstractCircuit,
    >>>     context: cirq.TransformerContext,
    >>>     *,
    >>>     atol: float = 1e-8,
    >>>     sqrt_iswap_gate: irq.ISwapPowGate = cirq.SQRT_ISWAP_INV,
    >>>     cleanup_operations: bool = True,
    >>> ) -> cirq.Circuit:
    >>>     pass

This is very useful as it gets rid of the originally discussed constraint of writing a transformer as a class only because it expects additional arguments which can't be supported due to limitations in mypy.

cc @maffoo #4483

@tanujkhattar
Copy link
Collaborator Author

Updated to also change context to a keyword argument and use it's default value declared assigned in the method signature, if none provided.

Fixes #4879

@CirqBot CirqBot added the size: M 50< lines changed <250 label Jan 25, 2022
@tanujkhattar
Copy link
Collaborator Author

@maffoo This is ready for re-review. PTAL

@tanujkhattar tanujkhattar added automerge Tells CirqBot to sync and merge this PR. (If it's running.) front_of_queue_automerge CirqBot uses this label to indicate (and remember) what's being merged next. labels Jan 28, 2022
@CirqBot
Copy link
Collaborator

CirqBot commented Jan 28, 2022

Automerge cancelled: Failed to update branch (incorrect expected_head_sha).

@CirqBot CirqBot removed automerge Tells CirqBot to sync and merge this PR. (If it's running.) front_of_queue_automerge CirqBot uses this label to indicate (and remember) what's being merged next. labels Jan 28, 2022
@tanujkhattar tanujkhattar merged commit fdf85f0 into quantumlib:master Jan 28, 2022
@tanujkhattar tanujkhattar deleted the default_args_in_transformer_api branch January 28, 2022 20:07
rht pushed a commit to rht/Cirq that referenced this pull request May 1, 2023
…PI. (quantumlib#4890)

* Support additional **kwargs with default arguments in Transformer API.

* Make context a keyword argument and use it's default value from signature, if provided

* Make context an optional parameter with default value as None
harry-phasecraft pushed a commit to PhaseCraft/Cirq that referenced this pull request Oct 31, 2024
…PI. (quantumlib#4890)

* Support additional **kwargs with default arguments in Transformer API.

* Make context a keyword argument and use it's default value from signature, if provided

* Make context an optional parameter with default value as None
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
size: M 50< lines changed <250
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Fix @cirq.transformer decorator to work with default value of context, if provided.
3 participants