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

Use @dataclass for cirq.google.[De]SerializingArg #2494

Merged
merged 8 commits into from
Nov 6, 2019
Merged

Use @dataclass for cirq.google.[De]SerializingArg #2494

merged 8 commits into from
Nov 6, 2019

Conversation

Strilanc
Copy link
Contributor

@Strilanc Strilanc commented Nov 5, 2019

No description provided.

@googlebot googlebot added the cla: yes Makes googlebot stop complaining. label Nov 5, 2019
"""Specification of the arguments to deserialize an argument to a gate.

Attributes:
Args:
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I realize this looks silly. But in my experimentation sphynx only seemed to understand the Args section, not the Attributes section.

Copy link
Collaborator

Choose a reason for hiding this comment

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

you could try digging into the napoleon documentation to see if there's some flag you need to set. IIRC napoleon (used for turning google-style docstrings into sphinx/rst) has a lot of flags you can set

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Reverted attributes->args change (will address in later PR if needed)

Copy link
Collaborator

@mpharrigan mpharrigan left a comment

Choose a reason for hiding this comment

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

love dataclasses

('gate_getter',
Union[str, Callable[['cirq.Gate'], arg_func_langs.ArgValue]]),
('required', bool)])):
@dataclass
Copy link
Collaborator

Choose a reason for hiding this comment

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

consider @dataclass(frozen=True) for immutability and hashability

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done.

@Strilanc Strilanc added the automerge Tells CirqBot to sync and merge this PR. (If it's running.) label Nov 6, 2019
@CirqBot
Copy link
Collaborator

CirqBot commented Nov 6, 2019

Automerge cancelled: A status check is failing.

@CirqBot CirqBot removed the automerge Tells CirqBot to sync and merge this PR. (If it's running.) label Nov 6, 2019
@Strilanc Strilanc added the automerge Tells CirqBot to sync and merge this PR. (If it's running.) label Nov 6, 2019
@CirqBot CirqBot added the front_of_queue_automerge CirqBot uses this label to indicate (and remember) what's being merged next. label Nov 6, 2019
@CirqBot
Copy link
Collaborator

CirqBot commented Nov 6, 2019

Automerge cancelled: There are merge conflicts.

@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 Nov 6, 2019
@Strilanc Strilanc added the automerge Tells CirqBot to sync and merge this PR. (If it's running.) label Nov 6, 2019
@CirqBot CirqBot added the front_of_queue_automerge CirqBot uses this label to indicate (and remember) what's being merged next. label Nov 6, 2019
@CirqBot CirqBot merged commit 9b51176 into master Nov 6, 2019
@CirqBot CirqBot deleted the doczar3 branch November 6, 2019 04:04
@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 Nov 6, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
cla: yes Makes googlebot stop complaining.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants