Skip to content

fix: handle reordered contents in ak.almost_equal #2424

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

Merged
merged 3 commits into from
Apr 25, 2023

Conversation

agoose77
Copy link
Collaborator

@agoose77 agoose77 commented Apr 25, 2023

ak.almost_equal made some poor assumptions about content orderings, which this PR fixes. Now, unions are considered equal against unions with different content orders (as long as the tags align equally). Records are compared by field for non-tuples, addressing the same problem with records.

@agoose77 agoose77 changed the title fix: almost_equal fix: handle reordered contents in ak.almost_equal Apr 25, 2023
@agoose77 agoose77 marked this pull request as ready for review April 25, 2023 09:32
@agoose77 agoose77 requested a review from jpivarski April 25, 2023 09:32
@agoose77 agoose77 temporarily deployed to docs-preview April 25, 2023 09:43 — with GitHub Actions Inactive
Copy link
Member

@jpivarski jpivarski left a comment

Choose a reason for hiding this comment

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

I see that this was pulled out of #2365 (comment)

Approved!

We can skip the codecov that didn't run. Maybe we should make it a non-required test, considering how often it fails to start.

@jpivarski
Copy link
Member

I removed codecov as a required test. (It was a weak requirement anyway; the test coverage would have to drop way down for it to fail.)

@agoose77 agoose77 merged commit 334e6b2 into main Apr 25, 2023
@agoose77 agoose77 deleted the agoose77/fix-almost-equals branch April 25, 2023 18:50
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.

2 participants