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

Very small clean up in the count_ops() method #4002

Closed
wants to merge 9 commits into from
Closed

Very small clean up in the count_ops() method #4002

wants to merge 9 commits into from

Conversation

rajkk1
Copy link
Contributor

@rajkk1 rajkk1 commented Mar 20, 2020

Summary

Very small clean up in the count_ops() method in the QuantumCircuit class. No change in functionality.

Details and comments

The clean up made the code slightly shorter and (in my opinion) slightly more readable. I am working on a different PR and thought of this small optimization so I thought I would create a separate PR for this before submitting my bigger PR. Thanks!

@CLAassistant
Copy link

CLAassistant commented Mar 20, 2020

CLA assistant check
All committers have signed the CLA.

Copy link
Member

@mtreinish mtreinish left a comment

Choose a reason for hiding this comment

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

Thanks for the PR, this is a good change and will actually improve performance of the method (doing get is more efficient because it doesn't have to traverse all the keys in the dict to find membership) but it will only probably be noticeable for a large number of different operations. Once you fix the lint errors I think this is good to go

@rajkk1
Copy link
Contributor Author

rajkk1 commented Mar 20, 2020

Thank you! I've fixed all the errors except for a single"invalid-file-header" error. I am running this on Windows 10 so I wonder if there is some weirdness going on there. I checked this thread #3127 but I couldn't really resolve the problem. Have you come across this before?

@rajkk1
Copy link
Contributor Author

rajkk1 commented Mar 23, 2020

Hi @mtreinish, hope you had a good weekend! Just wondering if you had a chance to look at my comment above. Thank you!

@mtreinish
Copy link
Member

@rajkk1 looking at the current version of this branch I do not think you're hitting an environment difference with pylint. The failures with the lint job here are from pycodestyle which does not have the same issues as pylint from that issue. You've removed all the blank lines from the class you were editing and added a blank line to the end. I would recommend reverting those changes on the branch git revert 53e86a1cde905704d00037087dfd8b0c28f82bca and git revert cc3b4325ed23637be9b8d879b290a96c0fb16acc (which are causing the doc job failure) and then look at the failure reported by CI for lint and manually fix those. It's just a mix of tabs and spaces for indents and whitespace in blank lines.

@rajkk1 rajkk1 requested a review from chriseclectic as a code owner March 23, 2020 20:48
@rajkk1
Copy link
Contributor Author

rajkk1 commented Mar 23, 2020

thank you @mtreinish! I tried reverting and it still wasn't happy. This is getting a bit confusing so I might just re-do the whole pull request to make it easier

@rajkk1 rajkk1 closed this Mar 23, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants