Skip to content

Conversation

joesho112358
Copy link
Contributor

Reading through, I noticed some things could be combined to reduce reading and indentation.

@joesho112358 joesho112358 requested review from a team, vtomole and cduck as code owners February 9, 2023 08:31
@joesho112358 joesho112358 requested a review from verult February 9, 2023 08:31
Copy link
Collaborator

@viathor viathor left a comment

Choose a reason for hiding this comment

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

Thank you for the PR! I agree two of the changes improve the code. Before merging, I would like to ask you to revert one of them to avoid repeated checks in a loop, see below.

active.remove(q)
elif q in active:
end_frontier[q] = cur_moment
active.remove(q)
Copy link
Collaborator

Choose a reason for hiding this comment

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

This is shorter, but tests the condition on each iteration. I think the previous code is slightly better.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

makes sense to me! pushed.

Copy link
Collaborator

@viathor viathor left a comment

Choose a reason for hiding this comment

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

Thank you!

@CirqBot CirqBot added the Size: XS <10 lines changed label Feb 12, 2023
@viathor viathor merged commit dd2667a into quantumlib:master Feb 12, 2023
harry-phasecraft pushed a commit to PhaseCraft/Cirq that referenced this pull request Oct 31, 2024
* reducing some indenting and amount of reading by changing some logic around in circuit.py

* reverting change per code review

---------

Co-authored-by: Adam Zalcman <40255865+viathor@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Size: XS <10 lines changed
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants