-
Notifications
You must be signed in to change notification settings - Fork 33
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 a wrapper for C.H5Lexists for the CommonFG, returns nil if link e… #57
Conversation
…xists, error if not
This GolangCI failure: I assume it is a CI setup error rather than code error? |
That failure looks to be related to golangci/golangci#35. I'm not sure how to deal with this since we are completely tied to Cgo with this repo. |
This should be fixed by #60. |
Great! Looking forward to contributing!
…On Tue, Sep 17, 2019 at 6:36 PM Dan Kortschak ***@***.***> wrote:
This should be fixed by #60 <#60>.
—
You are receiving this because you authored the thread.
Reply to this email directly, view it on GitHub
<#57?email_source=notifications&email_token=AEOWLFTFAOITRDH5FCKBSLLQKFLWXA5CNFSM4IXQ5BMKYY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOD66DPWQ#issuecomment-532428762>,
or mute the thread
<https://github.com/notifications/unsubscribe-auth/AEOWLFUUURFDQFFOQRGQZUTQKFLWXANCNFSM4IXQ5BMA>
.
|
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.
I think a bool is better here. Possibly with an error as well, but probably not.
Sure will do tomorrow
…On Tue, Sep 17, 2019 at 10:31 PM Dan Kortschak ***@***.***> wrote:
***@***.**** 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
<#57 (comment)>:
> @@ -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
<#57?email_source=notifications&email_token=AEOWLFSWSXQ3MBC25HEBJO3QKGHJPA5CNFSM4IXQ5BMKYY3PNVWWK3TUL52HS4DFWFIHK3DMKJSXC5LFON2FEZLWNFSXPKTDN5WW2ZLOORPWSZGOCFBZEOI#pullrequestreview-289641017>,
or mute the thread
<https://github.com/notifications/unsubscribe-auth/AEOWLFTEIBWD65EL45YZJGTQKGHJPANCNFSM4IXQ5BMA>
.
|
@kortschak Hi I make the changes, I also merged it to the other PR regarding the images where this feature is used |
Just pushed all the requested changes
…On Thu, 19 Sep 2019 at 19:22, Dan Kortschak ***@***.***> wrote:
***@***.**** commented on this pull request.
------------------------------
In h5g_group_test.go
<#57 (comment)>:
> @@ -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
<#57?email_source=notifications&email_token=AEOWLFQ7ZYL6RLOAMZ5V2HLQKQCT5A5CNFSM4IXQ5BMKYY3PNVWWK3TUL52HS4DFWFIHK3DMKJSXC5LFON2FEZLWNFSXPKTDN5WW2ZLOORPWSZGOCFLKMCI#discussion_r326420781>,
or mute the thread
<https://github.com/notifications/unsubscribe-auth/AEOWLFSLKOG56SW2ZBS3DZ3QKQCT5ANCNFSM4IXQ5BMA>
.
|
Thanks |
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 ***@***.***> wrote:
Thanks
—
You are receiving this because you authored the thread.
Reply to this email directly, view it on GitHub
<#57?email_source=notifications&email_token=AEOWLFWC2CBHHKMN477DVEDQKQPCPA5CNFSM4IXQ5BMKYY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOD7FHWRI#issuecomment-533363525>,
or mute the thread
<https://github.com/notifications/unsubscribe-auth/AEOWLFWFMOYILKD4UJUDAGTQKQPCPANCNFSM4IXQ5BMA>
.
|
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). |
Added the C.H5Lexists to check if the link(group, dataset, link) exists
Please take a look.