-
Notifications
You must be signed in to change notification settings - Fork 389
refactor: update TaskMetadata #1076
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
Conversation
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.
This looks great, thanks Martin! Also seems like it didn't take too long.
Yea we will def. have a merge conflict with #1070.
However, it seems like we can mostly resolve that by branching from that and re-applying the change (which is great!)
Will just add @isaac-chung and @gowitheflow-1998 as reviewers here as well.
Haha, yeah, if I were proficient with GritQL, it would definitely be fast. Took me a couple of hours, but much faster next time. |
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.
looks great! Thanks for updating these. Adding modalities
and changing from text_creation
to sample_creation
definitely makes it easier for merging meta data from the image branch and other modalities in the future. Agree with stats
-> descriptive_statistics
otherwise not much to add on my end.
96d5d2f
to
aeae8ce
Compare
@KennethEnevoldsen I've added a few points (6). Feel free to lower the amount, and then it's ready to merge 👍 |
@KennethEnevoldsen In case of merge-conflicts, we can just re-use the
.grit
script and re-apply using:Fixes #1054.
Likely discovered a bug in GritQL, report here.