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

Fix PEP8 code style and misspelling issues #53

Merged
merged 4 commits into from
May 4, 2021
Merged

Conversation

vxfield
Copy link
Member

@vxfield vxfield commented Apr 29, 2021

This PR fixes hundreds of PEP8/Pylint and some misspelling issues in preparation #50 (enforce code style guidelines in the CI/CD pipeline)

@vxfield vxfield requested review from guenp, sanjgupt and adelebai April 29, 2021 20:13
@vxfield
Copy link
Member Author

vxfield commented Apr 29, 2021

I still need to do more validations and re-run all tests, but at least most of applicable PEP8 issues were fixe.

I tried to be consistent on line breaks, but I may have missed a few things.

There were some exceptions that we can discuss, specially regarding long lines that contains URLs.

Copy link
Contributor

@sanjgupt sanjgupt left a comment

Choose a reason for hiding this comment

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

I have taken a quick look but I will go through it again, later today and run the tests as well.

blob_client = BlobClient.from_blob_url(
self.details.output_data_uri)
blob_uri = self.workspace._get_linked_storage_sas_uri(
blob_client.container_name, blob_client.blob_name)
payload = download_blob(blob_uri)
Copy link
Contributor

@sanjgupt sanjgupt Apr 30, 2021

Choose a reason for hiding this comment

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

I think we should run pycodestyle or flake8 on this repo and it will point out pep8 style issues as well.

Copy link
Contributor

@sanjgupt sanjgupt Apr 30, 2021

Choose a reason for hiding this comment

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

To autofix issues 'black' is a tool to help. It isn;t 100% pep8 compliant but there is way around it or autopep8or yapf

Copy link
Member Author

Choose a reason for hiding this comment

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

Yes, we have another issue #50 that will address that.
For now, I manually ran and fixed the issues using pylint and flake8.

Copy link
Member Author

Choose a reason for hiding this comment

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

Did you find a specific style issue with the highlighted code?
Is there a better way to break those lines so they will be under 80 chars?

blob_client = BlobClient.from_blob_url(
self.details.output_data_uri)
blob_uri = self.workspace._get_linked_storage_sas_uri(
blob_client.container_name, blob_client.blob_name)
payload = download_blob(blob_uri)
Copy link
Contributor

@sanjgupt sanjgupt Apr 30, 2021

Choose a reason for hiding this comment

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

I think we should run pycodestyle or flake8 on this repo and it will point out pep8 style issues as well.

@vxfield
Copy link
Member Author

vxfield commented May 1, 2021

Issue #50 that will address running linting tools on the repo with the CI/CD pipeline

@vxfield vxfield marked this pull request as ready for review May 1, 2021 01:43
@vxfield
Copy link
Member Author

vxfield commented May 1, 2021

I ran the unit tests and code is running fine.
I also fixed a credential scan false positive.
This is now ready for final review + merge.

@vxfield vxfield mentioned this pull request May 3, 2021
* auto correction with yapf and black

* styling for job.py

* pep8 for workspace.py

* pep8 storage.py

* pep8: streaming_problem

* pep8: solvers.py

* pep8 problem.py

* pep8: termp.py

* pep8: toshiba/solvers.py

* pep8: onequibit/solvers

* clean up

* pep8 init

* setup.py

* nit
@sanjgupt sanjgupt self-requested a review May 4, 2021 00:16
@vxfield vxfield enabled auto-merge (squash) May 4, 2021 00:16
@vxfield vxfield merged commit 335696d into main May 4, 2021
@vxfield vxfield deleted the xfield/code-style-fixes branch August 26, 2021 22:24
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.

2 participants