Skip to content
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

Add filter keyword argument to create_dataset #840

Merged
merged 7 commits into from
Jun 16, 2021
Merged

Add filter keyword argument to create_dataset #840

merged 7 commits into from
Jun 16, 2021

Conversation

david-macmahon
Copy link
Contributor

This allows create_dataset to create a dataset with a user-defined
filter by setting the filter keyword to a Tuple of integers that
describe and configure the filter. For example, to use a filter
specified by filter_id=32013, flags=0, cd_nelmts=3, and
cd_values=(2,0,8):

ds, dt = create_dataset(h5, "data", data,
                        chunk=(256,1,256),
                        filter=(32013,0,2,0,8))

Note that cd_nelmts is not included explicitly. It is inferred from
the length of the Tuple.

This allows `create_dataset` to create a dataset with a user-defined
filter by setting the `filter` keyword to a Tuple of integers that
describe and configure the filter.  For example, to use a filter
specified by `filter_id=32013`, `flags=0`, `cd_nelmts=3`, and
`cd_values=(2,0,8)`:

    ds, dt = create_dataset(h5, "data", data,
                            chunk=(256,1,256),
                            filter=(32013,0,2,0,8))

Note that `cd_nelmts` is not included explicitly.  It is inferred from
the length of the Tuple.
This is useful for user-defined filters.
The `filter` keyword argument now accepts `Tuple{Tuple{<:Integer}}` to
enable multiple user-defined filters when creating a dataset.  Passing
`Tuple{<:Integer}` for a single user-defined filter is still supported.
src/HDF5.jl Outdated Show resolved Hide resolved
src/HDF5.jl Outdated Show resolved Hide resolved
@musm
Copy link
Member

musm commented Jun 15, 2021

Looks like there's some issue with CI (might need to rebase on top of master), but otherwise LGTM.

@musm musm requested a review from jmert June 15, 2021 04:59
@musm
Copy link
Member

musm commented Jun 15, 2021

will merge in 24 hours sans objections

Better names are better! :)
@david-macmahon
Copy link
Contributor Author

I just pushed the suggested changes (though not via the GitHub interface). As the commit log message says, better names are better. Thanks!

@musm
Copy link
Member

musm commented Jun 15, 2021

@musm
Copy link
Member

musm commented Jun 15, 2021

Looks like it's a convert error although I'm not sure why it's isolated to julia 1.3

  MethodError: no method matching unsafe_convert(::Type{Ptr{UInt32}}, ::Base.RefValue{Tuple{UInt32}})

src/HDF5.jl Outdated Show resolved Hide resolved
This new way actually allocates fewer bytes and is compatible with
Julia 1.3.
src/HDF5.jl Show resolved Hide resolved
@david-macmahon
Copy link
Contributor Author

Thanks for your great detective work @musm!

@musm musm merged commit 834141a into JuliaIO:master Jun 16, 2021
@simonbyrne simonbyrne mentioned this pull request Jun 21, 2021
musm pushed a commit that referenced this pull request Oct 27, 2021
* Add `filter` keyword argument to create_dataset

This allows `create_dataset` to create a dataset with a user-defined
filter by setting the `filter` keyword to a Tuple of integers that
describe and configure the filter.  For example, to use a filter
specified by `filter_id=32013`, `flags=0`, `cd_nelmts=3`, and
`cd_values=(2,0,8)`:

    ds, dt = create_dataset(h5, "data", data,
                            chunk=(256,1,256),
                            filter=(32013,0,2,0,8))

Note that `cd_nelmts` is not included explicitly.  It is inferred from
the length of the Tuple.

* Add H5Z_FLAG_MANDATORY constant

This is useful for user-defined filters.

* Support multiple user-defined filters per dataset

The `filter` keyword argument now accepts `Tuple{Tuple{<:Integer}}` to
enable multiple user-defined filters when creating a dataset.  Passing
`Tuple{<:Integer}` for a single user-defined filter is still supported.

* Add tests for filter keyword arg of create_dataset

* Change some type/names based on code review

Better names are better! :)

* Change how `set_filter` passes Tuple as Ptr{Cuint}

This new way actually allocates fewer bytes and is compatible with
Julia 1.3.

* Add comment about passing cd_values in set_filter

(cherry picked from commit 834141a)
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