-
Notifications
You must be signed in to change notification settings - Fork 55
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
Replace unnecessary calls to .flatten()
with .ravel()
.
#431
Comments
Sounds great! |
@rileyjmurray Could you explain a bit more when .ravel() can be used instead of .flatten()? I'm confused about when a copy is necessary. |
@pcwysoc, as a rule of thumb, copies aren't necessary. Python (or rather, numpy) makes them all the time. The main time when a copy would be important is if you're explicitly using in-place operations or if you're overwriting portions of an array. import numpy as np
a = np.ones((3,5))
b = a.ravel()
b = b + 2
print(a) # unchanged, because the change to b1 wasn't specifically in-place.
b = a.ravel()
b += 2
print(a) # changed, since "+=" is an in-place operation
a = np.ones((3,5))
b = a.ravel()
b[::2] = 0.0 # overwrite every other element of b with 0.0.
print(a) # changed, since b shares memory with a |
I've started working on this. Here are common patterns I've seen where .flatten() can be safely replaced with .ravel():
I'll edit this comment as I go. |
Fixed in #451 |
As of the latest commit on develop (between releases of pyGSTi 0.9.12 and 0.9.13)m there are 102 hits for
.flatten()
in the pyGSTi codebase.The documentation for flatten says that this returns a flattened copy of the input array. In the vast majority of situations it's preferable to use
.ravel()
instead, which only returns a copy when necessary. I'll make a PR in the near future for these changes.This is spiritually similar to #429 and #430, but different enough to warrant its own tracking issue.
The text was updated successfully, but these errors were encountered: