-
-
Notifications
You must be signed in to change notification settings - Fork 330
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. |
4fccfb8
to
798132f
Compare
Fixes #3050
Core Fix in
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 downstreamPlease let me know if there is any changes needed in the issue fix.
TODO:
docs/user-guide/*.rst
changes/