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
Tag htype #2676
Tag htype #2676
Conversation
Codecov ReportAttention:
Additional details and impacted files@@ Coverage Diff @@
## main #2676 +/- ##
==========================================
- Coverage 84.37% 83.81% -0.56%
==========================================
Files 231 231
Lines 25902 25914 +12
==========================================
- Hits 21854 21720 -134
- Misses 4048 4194 +146
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. |
I do get all of the changes, but I don't get bigger picture, why do we need tag htype? From what I am seeing it is the same text htype but with a different name. If there is a specific reason on why we need it, maybe then it would be a better idea to create a base class for the htypes that are string? |
Kudos, SonarCloud Quality Gate passed! |
🚀 🚀 Pull Request
Impact
Description
class_label
because there's no need to encode the tags as numbers and the htype's name is not intuitive.text
orjson
because these htypes need to support large amounts of information in comparison to tags, and visualizer needs to display both of these differently.Things to be aware of
Things to worry about
Additional Context