BUG: Fix bug in numpy.pad() #25963
BUG: Fix bug in numpy.pad() #25963
Conversation
|
Looks good to me! The code for |
|
Please squash on merge. |
See numpy#25926 for the bug Fix linter bug (E501 line too long)
EngineerEricXie
force-pushed
the
pad_debug
branch
from
fdfac08 to
0c22b67
Compare
|
@charris I have squashed the commits and force pushed the branch for this PR. As it's my first PR, I want to ensure I've followed the correct process. Please let me know if there's anything incorrect or if I've missed any steps. Thanks！ |
ngoldbaum
added
the
triage review
|
@lagru do you have time to take a brief look (probably for along). IIRC you were the last one who looked at this. |
ngoldbaum
added
triaged
|
@seberg, sorry for the late response. I'm not sure when I will get to it in the next few days, but it's on my list. |
numpy/lib/_arraypad_impl.py
Outdated
| # by ensuring period can only be a multiple of the original | ||
| # area's length. | ||
| old_length = (old_length - 1) // (original_period - 1) \ | ||
| * (original_period - 1) + 1 |
There was a problem hiding this comment.
Looks like the + 1 is subtracted out in the second line below. This line should also be indented for readability.
There was a problem hiding this comment.
Instead of line continuation, you can put the whole right hand side in parenthesis.
|
@charris I have made some modifications according to your suggestions. However, one check regarding FreeBSD has failed. Since I only modified a single line of code, I am curious about how I can fix this problem. Could you provide some suggestions? Thank you! |
|
The FreeBSD failure is unrelated and is an infrastructure problem, not a code problem. |
|
@charris Thanks for replying! Do you think this PR is good to go, or is there anything else I should address? |
|
Looks like you have unintended changes, pulling in a couple of submodules among other things. Might want to redo the last commit. |
EngineerEricXie
force-pushed
the
pad_debug
branch
from
5340b61 to
839afea
Compare
|
@charris I have redone the last commit, removed the unintended changes, and force-pushed it. However, it caused some checks related to the GCC version to fail. Should this cause any trouble while merging? |
That is an unrelated error due to a recent merge. |
|
Thanks @EngineerEricXie . The code seems unnecessarily complex for the simple operation it implements, it would be nice to refactor it. I think a good starting point would be to move the iteration into |
* BUG: Fix bug in numpy.pad() and add test cases See numpy#25926 for the bug Fix linter bug (E501 line too long) * Increase readability and clarity
* BUG: Fix bug in numpy.pad() and add test cases See numpy#25926 for the bug Fix linter bug (E501 line too long) * Increase readability and clarity
See #25926 for the bug