-
-
Notifications
You must be signed in to change notification settings - Fork 329
Fix AsyncGroup.create_dataset() dtype handling and optimize tests #3050 #3059
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
base: main
Are you sure you want to change the base?
Conversation
pre-commit.ci autofix |
HI @d-v-b can you please review the PR. Also do I need to make separate tests for this code changes? |
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.
Perhaps two distinct PRs would have been preferable, but I'm not maintainer so you can keep it as is for now.
i'll look at this later today |
can you explain some of the changes to the hypothesis tests? |
@d-v-b When running tests locally, I was encountering issue with slow test running or test running more than the default deadline for some particular tests. I tried some changes with the fix but still it was the same. Hence I tried to avoid and optimize the test codes. |
@dhruvak001 can we see what happens in our CI tests if you roll back the changes to the hypothesis tests? |
@d-v-b yup it passes even after reverting changes. Thankyou. |
Fixes #3050
AsyncGroup.create_dataset()
:-- If
dtype
is not provided in the arguments-- And
if data is None
(meaning no data is being provided to infer the dtype from)-- Then raise a clear error message saying "
dtype
must be provided ifdata is None
"create_array()
always receives the requireddtype
parameter, preventing potential errors downstreamtest_properties.py
:-- Removed the time limit by setting
deadline=None
and suppressing the"too slow"
health check-- Reduced the complexity of test cases by:
-- Limiting arrays to maximum 3 dimensions (down from 4)
-- Setting maximum side length to 8 (to prevent very large arrays)
-- Adding assumptions to prevent repeated indices in test cases
-- For test_vindex, limiting the result shape to 2 dimensions to reduce complexity
Please let me know if there is any changes needed to the approach in the tests or the issue fix.
TODO:
docs/user-guide/*.rst
changes/