-
Notifications
You must be signed in to change notification settings - Fork 38
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
Run tests with h5py in workflow #193
Conversation
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.
lgtm other than the comments
.github/workflows/main.yml
Outdated
env: | ||
HS_USERNAME: test_user1 | ||
HS_PASSWORD: test | ||
TEST2_USERNAME: test_user1 |
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.
Should be test_user2
HS_USERNAME: test_user1 | ||
HS_PASSWORD: test | ||
TEST2_USERNAME: test_user1 | ||
TEST2_PASSWORD: test |
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.
here too
A few changes to the tests are necessary for this workflow to pass:
test_index_emptylist
is expected to fail because h5pyd returns an empty array with no shape, and numpy returns an empty array with the shape(0, 3)
. Now, when h5pyd is returning an empty selection, it sets the 'shape' equal to what would result from executing the same (empty) selection on an ndarray.test_ellipsis
andtest_tuple
underTestScalarArray
are expected to raise an error due toH5T_ARRAY
only being partially implemented in HSDS. For now, if h5py is being used, these tests manually raise an error to be consistent.is_hdf5
on an ID when using h5py, since the h5py implementation of that function expects a string path, not an integer ID.