-
Notifications
You must be signed in to change notification settings - Fork 603
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
unpin black, make code black-friendly #5119
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
these are the only manual changes I had to make. for some reason they all just became outdated 😵💫 but this is the list
pennylane/data/attributes/list.py
Outdated
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
the versions disagreed on formatting empty functions with ...
, but using pass
should be the same
pennylane/data/base/attribute.py
Outdated
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
same problem with ...
vs pass
coeff = np.sqrt(2 ** (1 - num_op_wires)) | ||
vector = np.array( | ||
[ | ||
np.sqrt(2 ** (1 - num_op_wires)) | ||
* np.cos(-np.pi / 2 + np.pi * x / 2**num_op_wires) | ||
coeff * np.cos(-np.pi / 2 + np.pi * x / 2**num_op_wires) | ||
for x in range(2**num_op_wires) | ||
] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
factored out the coeff to make it work
pennylane/ops/channel.py
Outdated
common_term = np.sqrt( | ||
4 * eT2**2 | ||
+ 4 * p_reset**2 * pe**2 | ||
- 4 * p_reset**2 * pe | ||
+ p_reset**2 | ||
+ np.eps | ||
base = sum( | ||
4 * eT2**2, | ||
4 * p_reset**2 * pe**2, | ||
-4 * p_reset**2 * pe, | ||
p_reset**2, | ||
np.eps, | ||
) | ||
common_term = np.sqrt(base) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
idk why but this worked
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
put the for
logic in each branch
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
factored out the base
in each test to make it happy
# pylint: disable=import-outside-toplevel,too-many-branches,not-callable,unexpected-keyword-arg | ||
# pylint: disable=unused-argument,unnecessary-lambda-assignment,inconsistent-return-statements | ||
# pylint: disable=invalid-unary-operand-type,isinstance-second-argument-not-valid-type | ||
# pylint: disable=too-many-arguments,too-many-statements,function-redefined,too-many-function-args |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
😭
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
i know... i just rearranged it, only added 2. but still
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Think I'm actually a fan of the new syntax for multi-line comprehensions.
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## master #5119 +/- ##
==========================================
- Coverage 99.68% 99.47% -0.22%
==========================================
Files 391 393 +2
Lines 35793 35713 -80
==========================================
- Hits 35682 35524 -158
- Misses 111 189 +78 ☔ View full report in Codecov by Sentry. |
the new changes from black have thrown codecov all out of sorts, will need a force-merge upon 2nd approval |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks @timmysilv !
Context:
See #5112. New version of
black
came out, and it auto-formatted a bunch of files. Most of these changes are backwards-compatible (with previous versions ofblack
) but not all, so it was non-trivialDescription of the Change:
Did the following:
pip install -U black make format git add . pip install black~=23.12 make format
At this point, all unstaged changes are non-backwards-compatible lines. So, I tweaked those lines, ran
make format
to make sure they were good with old black, then re-ran the above set of commands to ensure it's forwards-compatible too. I'll inline comment each actual change I made.Benefits:
black
they choose, including the new onePossible Drawbacks:
N/A
[sc-55823]