-
-
Notifications
You must be signed in to change notification settings - Fork 780
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
Find and fix typos with codespell #8304
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.
Thanks @cclauss, IIRC you made this type of PR in the past to PyWavelets and/or scikit-image. I have been a fan of using codespell since that time.
Aside from the COO issue already discussed, I commented on just one other line where the suggestion should not be applied.
cupy/lib/_shape_base.py
Outdated
@@ -76,7 +76,7 @@ def _make_along_axis_idx(arr_shape, indices, axis): | |||
dest_dims = list(range(axis)) + [None] + \ | |||
list(range(axis + 1, indices.ndim)) | |||
|
|||
# build a fancy index, consisting of orthogonal aranges, with the | |||
# build a fancy index, consisting of orthogonal arranges, with the |
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.
This change should not be made. "aranges" is refering to multiple calls to cupy.arange
in the loop below.
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.
One option could be to refer to them as just "orthogonal ranges". One might associate "range" with Python's built-in range
rather than cupy.arange
, but I think the meaning would still be clear enough given the actual code within the loop.
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.
orthogonal cupy.arange calls
Hi @cclauss I noticed the PR was closed and the branch was deleted, do you think it's something you still want to push for, or should we take over and revive it? |
@leofang I re-opend if you want to merge or cherry-pick parts. |
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.
Hi @cclauss, sorry for taking the time to get back to you! I've checked all the changes one by one. Looks great except for a few commented!
Co-authored-by: Kenichi Maehashi <939877+kmaehashi@users.noreply.github.com>
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, LGTM!
/test mini |
Head branch was pushed to by a user without write access
@kmaehashi Failed to backport automatically.
|
Find and fix typos with codespell
@kmaehashi As discussed elsewhere...