search

gonum / hdf5

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

Add a wrapper for C.H5Lexists for the CommonFG, returns nil if link e… #57

Closed zyc-sudo closed 4 years ago

zyc-sudo commented 4 years ago

Added the C.H5Lexists to check if the link(group, dataset, link) exists Please take a look.

zyc-sudo commented 4 years ago

This GolangCI failure:
compilation terminated.

gonum.org/v1/hdf5

./cgoflags.go:12:19: fatal error: hdf5.h: No such file or directory

I assume it is a CI setup error rather than code error? // #include "hdf5.h"

kortschak commented 4 years ago

That failure looks to be related to https://github.com/golangci/golangci/issues/35. I'm not sure how to deal with this since we are completely tied to Cgo with this repo.

kortschak commented 4 years ago

This should be fixed by #60.

zyc-sudo commented 4 years ago

Great! Looking forward to contributing!

On Tue, Sep 17, 2019 at 6:36 PM Dan Kortschak notifications@github.com wrote:

This should be fixed by #60 https://github.com/gonum/hdf5/pull/60.

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

zyc-sudo commented 4 years ago

Sure will do tomorrow

On Tue, Sep 17, 2019 at 10:31 PM Dan Kortschak notifications@github.com wrote:

@kortschak commented on this pull request.

I think a bool is better here. Possibly with an error as well, but probably not.

In h5g_group.go https://github.com/gonum/hdf5/pull/57#discussion_r325459540:

@@ -162,3 +163,15 @@ func (g Group) CreateTableFrom(name string, dtype interface{}, chunkSize, compr func (g Group) OpenTable(name string) (*Table, error) { return openTable(g.id, name) } + +// CheckLink checks if a link( can be group or dataset or actual link )exsit or not. This method won't give you annoying hdf5 warnings when called in a goroutine +// CheckLink returns nil when a link exist, return an error when the link doesn't exist or some other error occured

// LinkExists returns whether a link with the specified name exists in the group. func (g *CommonFG) LinkExists(name string) bool { c_name := C.CString(name) defer C.free(unsafe.Pointer(c_name)) return C.H5Lexists(g.id, c_name, 0) > 0 }

The HDF5 API is discusting for this. The return status, according to this https://support.hdfgroup.org/HDF5/doc/RM/RM_H5L.html#Link-Exists, is almost entirely uninformative for the failing case, so I'm not sure that it's even worth bothering to indicate the error return. @sbinet https://github.com/sbinet?

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

zyc-sudo commented 4 years ago

@kortschak Hi I make the changes, I also merged it to the other PR regarding the images where this feature is used

zyc-sudo commented 4 years ago

Just pushed all the requested changes

On Thu, 19 Sep 2019 at 19:22, Dan Kortschak notifications@github.com wrote:

@kortschak commented on this pull request.

In h5g_group_test.go https://github.com/gonum/hdf5/pull/57#discussion_r326420781:

@@ -28,6 +32,14 @@ func TestGroup(t *testing.T) { t.Errorf("wrong Name for group: want %q, got %q", "/foo", g1.Name()) }

  • if !f.LinkExists("/foo") {
  • t.Fatalf("err: %s", "/foo should exist at this time")

Yes. Thanks.

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

kortschak commented 4 years ago

Thanks

zyc-sudo commented 4 years ago

Thanks a lot! This is the first time I participated github open source. Thank you very much for your patience

On Thu, Sep 19, 2019 at 9:08 PM Dan Kortschak notifications@github.com wrote:

Thanks

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

kortschak commented 4 years ago

I neglected to say, would you please send a PR adding yourself to AUTHORS and CONTRIBUTORS at gonum/gonum. Please use the commit message "A+C: add Yucheng Zhu" (I assume this is your name from the commits).