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

Switch to using black for code formatting #81

Merged
merged 1 commit into from
May 5, 2021

Conversation

manoelmarques
Copy link
Contributor

Summary

Follows the changes made in Qiskit/qiskit#6361

Details and comments

@adekusar-drl
Copy link
Collaborator

This is a disaster! I can't say we had a very well formatted code, but this is terrible.
instead of

                    input_grad = grad_output.transpose(0, 1) @ input_grad.transpose(0, 1)

now:

                    input_grad = grad_output.transpose(0, 1) @ input_grad.transpose(
                        0, 1
                    )

Or:
instead of

                    weights_grad = weights_grad.to_dense()  # this should be eventually removed

it is

                    weights_grad = (
                        weights_grad.to_dense()
                    )  # this should be eventually removed

Or:

            parameterized_circuit = self._quantum_instance.transpile(parameterized_circuit)[0]

replaced with

            parameterized_circuit = self._quantum_instance.transpile(
                parameterized_circuit
            )[0]

and so on and so on...

@adekusar-drl
Copy link
Collaborator

Any ideas why max-line-length=105?

@manoelmarques
Copy link
Contributor Author

manoelmarques commented May 5, 2021

Terra changed to 105 from 100 in their PR. I don't know why but would think it has to do on how black formatted code which may have increased de size of the code in one line.

@woodsp-ibm
Copy link
Member

This issue in Terra has a note as to the line length of 105 Qiskit/qiskit#5883

I agree it does not do a great job in some places - I made a comment in the equivalent PR in Nature.

@adekusar-drl
Copy link
Collaborator

Thanks for pointing out to the Nature's PR. I agree with @mtreinish in general. Well, of course we get rid of any formatting questions but at what a cost.
I'd rather increase max line length to 140-150, we have wide screens for many years, and change the style.

@adekusar-drl
Copy link
Collaborator

Terra changed to 105 from 100 in their PR. I don't know why but would think it has to do on how black formatted code which may have increased de size of the code in one line.

This is the answer from Qiskit/qiskit#5883:

Black treats maximum-line-length "like a speed limit" ie if the least bad option is to exceed the line length by a little, then it will sometimes do so (I kept black using line length 100 and set the pylint/pycodestyle line length to 105 to accommodate this -- there are currently no places where this actually comes up in qiskit)

@manoelmarques manoelmarques merged commit 6c9a6fa into qiskit-community:main May 5, 2021
@manoelmarques manoelmarques deleted the black branch May 5, 2021 19:47
gentinettagian pushed a commit to gentinettagian/qiskit-machine-learning that referenced this pull request Dec 14, 2021
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