-
Notifications
You must be signed in to change notification settings - Fork 135
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
Hierarchical Labeling #742
Hierarchical Labeling #742
Conversation
Check out this pull request on See visual diffs & provide feedback on Jupyter Notebooks. Powered by ReviewNB |
7bec117
to
4ca506e
Compare
f3287d4
to
192820f
Compare
022f72a
to
2e117c5
Compare
2e117c5
to
7baa7c9
Compare
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.
But I don't still get the concept of this work. Our current data structure of categories
has a parent
entity. It means that it can represent any tree structure with arbitrary depth and width.
How about this case?
l1(T)
/ \
l2(F) l3(F)
/ \ / \
l4 l5 l6 l7
and Item1.annotations := [l1, l2, l3, l4, l5, l6, l7]
(if label
has single_selection=True
, then it denotes label(T)
, otherwise, label(F)
). Then, this filtering will make Item1.annotations = [l1, l2, l4, l5, l6, l7]
.
I feel this is a bit inconsistent because l6
and l7
is still alive although their parent l3
is filtered out.
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
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.
Could you check whether Vinnam's concern is true or not?
After facing some practical cases, we may change this filtering policy again. |
As vinnam suggested and discussed with wonju, I'll follow the LabelGroup implementation to mark the 'exclusive' after that I'll request review again. |
516e0c7
to
8eb3708
Compare
As we discussed, I import the LabelGroup format from the OTX, please review again. |
def8347
to
acea3b6
Compare
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.
Summary
Resolves 93482
How to test
Checklist
develop
branchLicense
Feel free to contact the maintainers if that's a concern.