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

Closes #3782: flip function to match numpy #3791

Merged

Conversation

ajpotts
Copy link
Contributor

@ajpotts ajpotts commented Sep 26, 2024

This PR adds a flip function to match numpy:
https://numpy.org/doc/2.1/reference/generated/numpy.flip.html

Closes #3782: flip function to match numpy

@ajpotts ajpotts force-pushed the 3782-flip-function-to-match-numpy branch 2 times, most recently from 97f0724 to 9e1f277 Compare September 27, 2024 13:49
@ajpotts ajpotts marked this pull request as ready for review September 27, 2024 14:12
@ajpotts ajpotts force-pushed the 3782-flip-function-to-match-numpy branch from 9e1f277 to 4a7b6a3 Compare September 30, 2024 21:14
@drculhane
Copy link
Contributor

When I download this and run pytest on test_manipulation_functions, test_flip_string fails in single dim, and both test_flip_string and test_flip_multi_dim_bool fail in multi dim. But I see that it passed all the checks, so I'm not sure what to make of this.

image

@ajpotts ajpotts marked this pull request as draft October 3, 2024 14:40
@ajpotts ajpotts force-pushed the 3782-flip-function-to-match-numpy branch 3 times, most recently from 70194c8 to 188e515 Compare October 3, 2024 17:01
@ajpotts
Copy link
Contributor Author

ajpotts commented Oct 3, 2024

Fixed two problems likely causing it to fail locally but not in the CI:

  • The tests had not been added to pytest.ini
  • test_flip_multi_dim_bool had poor logic that needed to be replace.

Thanks Andy, for pointing this out. @drculhane

@ajpotts ajpotts marked this pull request as ready for review October 3, 2024 17:03
@drculhane
Copy link
Contributor

When I download this and run pytest on test_manipulation_functions, test_flip_string fails in single dim, and both test_flip_string and test_flip_multi_dim_bool fail in multi dim. But I see that it passed all the checks, so I'm not sure what to make of this.

image

@drculhane
Copy link
Contributor

This is what I get. I had it sent to a text file, since these sorts of things are always long.

errors.txt

Copy link
Contributor

@drculhane drculhane left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good.

Copy link
Contributor

@jaketrookman jaketrookman left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good

Copy link
Member

@stress-tess stress-tess left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

looks good to me! small comment on the test class name

tests/numpy/manipulation_functions_test.py Outdated Show resolved Hide resolved
@ajpotts ajpotts force-pushed the 3782-flip-function-to-match-numpy branch from 188e515 to 6d011a2 Compare October 7, 2024 12:37
@ajpotts ajpotts force-pushed the 3782-flip-function-to-match-numpy branch from 1b91ab4 to a07e845 Compare October 7, 2024 12:43
@stress-tess stress-tess added this pull request to the merge queue Oct 7, 2024
Merged via the queue into Bears-R-Us:master with commit 5136a68 Oct 7, 2024
11 checks passed
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.

flip function to match numpy
4 participants