gonum / hdf5

hdf5 is a wrapper for the HDF5 library
BSD 3-Clause "New" or "Revised" License
131 stars 33 forks source link

Pull request to support dataset resize, HDF5 true colour image creation #54

Closed zyc-sudo closed 4 years ago

zyc-sudo commented 4 years ago

Hi, I added three functionalities to the hdf5:

  1. Added an utility of check if a node exist or not in the file Problem: Other methods, for example, "open a group and check the err to determine if a group exist" works great until the call are done in a goroutine. For me it is a gRPC server method. Even if I checked the error. The C will still complaining the error( HDF5-DIAG errors).These error message won't have any affect on the goroutine's functionality since we handled the but they contaminated my log. Check this post 10 years ago: http://hdf-forum.184993.n3.nabble.com/check-if-dataset-exists-td194725.html. Solution: looks like the best way is to use C's H5Lexists. It is safe, no HDF5-DIAG errors anymore, and works for group and dataset. So I added the following function in the h5g_group.go func (g *CommonFG) CheckLink(name string) error{}

  2. New support to create HDF5 true colour images Problem: there is no wrapper for adding a true colour images for a CommonFG.( true colour images can be viewed in HDF5 compass as image directly instead of array. Solution: Added the following function in the h5g_group.go func (g *CommonFG) CreateTrueImage(name string, img image.Image) error {}

  3. Added wrapper to resize a chunked dataset: Problem: there is no wrapper for resize a chunked dataset Solution: created the following function in h5d_dataset.go: func (s *Dataset) Resize(dims []uint) error {}

P.S. I changed a few imports from gonum.org/v1/hdf5 to github.com/zyc-sudo/hdf5 just to make it work in my environment. Please don't merge those.

Please let me know if there are any improvements I need to add.

Regards, Yucheng

kortschak commented 4 years ago

I'm sorry, we cant do anything with this because of the import line changes. If you wish to merge this back in the the gonum/hdf5 tree, you will need to work with your fork in the canonical path for hdf5.

If you do want to go ahead with this, we would appreciate it if you could make your PR history easier to follow as separate or separable commits. I would recommend this be three separate PRs each with only one commit, one for each of the features. For the second feature, I would expect there should also be a true image getter so that these can be tround-tripped.

zyc-sudo commented 4 years ago

Thank you very much. I’ll clean them up and implement the get image function and give you guys new PR .

On Thu, Sep 12, 2019 at 6:21 PM Dan Kortschak notifications@github.com wrote:

I'm sorry, we cant do anything with this because of the import line changes. If you wish to merge this back in the the gonum/hdf5 tree, you will need to work with your fork in the canonical path for hdf5.

If you do want to go ahead with this, we would appreciate it if you could make your PR history easier to follow as separate or separable commits. I would recommend this be three separate PRs each with only one commit, one for each of the features. For the second feature, I would expect there should also be a true image getter so that these can be tround-tripped.

— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub https://github.com/gonum/hdf5/pull/54?email_source=notifications&email_token=AEOWLFUMEK4IKYVCNDGFOTLQJK6GHA5CNFSM4IWKA742YY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOD6TN2IA#issuecomment-531029280, or mute the thread https://github.com/notifications/unsubscribe-auth/AEOWLFS25HQCSJXGZGMTL5LQJK6GHANCNFSM4IWKA74Q .

zyc-sudo commented 4 years ago

I opened 3 new PR to separate the features