-
Notifications
You must be signed in to change notification settings - Fork 89
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
refactor: add array comparison test helper #2024
Conversation
Codecov Report
Additional details and impacted files
|
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.
Note for posterity: we talked about this on Slack. But there are a few (good) additions since then, too.
You extract the two behaviors, but do you check them for equality? If so, I don't see it.
The "approximate dtype" is interesting: normally, we want types to be exact, but we do have an issue in which NumPy on Windows initializes integers as 32-bit. In our tests so far, we try to be diligent about explicitly setting the integer dtype to int64
when using NumPy, but this function would accept 32-bit integers and 64-bit integers as the same.
Overall, I approve! We'll probably be tweaking it as we use it, to more clearly define what we want to call equal same what we want to call not-equal.
No, I only check that they map to the same behaviour class for the array / record parameters. This might not be sufficient; we obviously also permit overloads of ufuncs, which wouldn't be caught this way. Equally, I wouldn't want to ban cases where additional behaviour classes were added to the behaviour dictionary that aren't used. Unless we consider that an desirable outcome?
Right, I'm worried about arrays_approx_equal(array, [1, 2, ...]) failing due to the default types for |
Add a helper function to
ak._util
that makes approximate comparisons of ragged arrays.Some of the branches in this helper may not have been tested; my view is that we'll discover any bugs when we use it in a test. I've checked all of the primitive cases, though.