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

chop respects axis order of args #1092

Merged
merged 3 commits into from
Jul 30, 2022
Merged

chop respects axis order of args #1092

merged 3 commits into from
Jul 30, 2022

Conversation

ddkohler
Copy link
Contributor

@ddkohler ddkohler commented Jul 29, 2022

Changes

I am intentionally omitting a changelog entry since this bug is not present in any version.

Checklist

  • added tests, if applicable
  • tests pass

ksunden
ksunden previously approved these changes Jul 29, 2022
@ksunden
Copy link
Member

ksunden commented Jul 29, 2022

Well, I thought it looked good, but tests are failing...

Initially wondered if it would handle the case of being passed ints in, but we make sure everything is strings at this point.

@ksunden ksunden dismissed their stale review July 29, 2022 22:16

tests failed

@ksunden
Copy link
Member

ksunden commented Jul 30, 2022

I think you took the opposite meaning from my comment...

I meant that I thought it was wrong initially, but upon closer inspection decided that it was correct because we translate the ints to strings.

I did not think that was the error causing tests to fail...

@ksunden ksunden merged commit 60e5e0c into master Jul 30, 2022
@ksunden ksunden deleted the chop-patch branch July 30, 2022 01:35
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

chop: respect axes order in method call
2 participants