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

Reducing some code by cleaning up some logic #6003

Merged
merged 3 commits into from
Feb 12, 2023
Merged

Reducing some code by cleaning up some logic #6003

merged 3 commits into from
Feb 12, 2023

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