-
Notifications
You must be signed in to change notification settings - Fork 138
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
Fix validator and add notebooks and document for level-up validator #933
Conversation
Check out this pull request on See visual diffs & provide feedback on Jupyter Notebooks. Powered by ReviewNB |
Codecov ReportPatch coverage:
Additional details and impacted files@@ Coverage Diff @@
## releases/1.2.0 #933 +/- ##
==================================================
- Coverage 78.64% 78.60% -0.05%
==================================================
Files 228 231 +3
Lines 26349 26397 +48
Branches 5252 5254 +2
==================================================
+ Hits 20723 20749 +26
- Misses 4407 4422 +15
- Partials 1219 1226 +7
Flags with carried forward coverage won't be shown. Click here to find out more.
... and 13 files with indirect coverage changes Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here. ☔ View full report in Codecov by Sentry. |
docs/source/docs/level-up/intermediate_skills/08_data_validate.rst
Outdated
Show resolved
Hide resolved
Signed-off-by: Kim, Vinnam <[email protected]>
datumaro/plugins/validators.py
Outdated
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.
Would you tell us what you aimed to achieve by changing the code in this file? Just for refactoring?
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.
Yes I did, I tried to reuse some functions by refactoring.
View / edit / reply to this conversation on ReviewNB vinnamkim commented on 2023-04-17T02:05:32Z Not to be fixed in this PR, but it seems that we need to discuss about dropping |
View / edit / reply to this conversation on ReviewNB vinnamkim commented on 2023-04-17T02:05:33Z We need to make a requirement for enhancing |
View / edit / reply to this conversation on ReviewNB vinnamkim commented on 2023-04-17T02:05:34Z It would be nice if the floating histogram was also provided by the Datumaro API. |
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
How to test
Checklist
License
Feel free to contact the maintainers if that's a concern.