Add deserializer class #37
Conversation
|
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #37 +/- ##
=======================================
Coverage ? 80.96%
=======================================
Files ? 34
Lines ? 1529
Branches ? 0
=======================================
Hits ? 1238
Misses ? 291
Partials ? 0
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
Hind-M
left a comment
Hind-M
left a comment
There was a problem hiding this comment.
Should we add deserializer to README and examples like serializer?
| // Verify | ||
| REQUIRE_EQ(batches.size(), 1); | ||
| CHECK_EQ(batches[0].nb_columns(), original_batch.nb_columns()); | ||
| CHECK_EQ(batches[0].nb_rows(), original_batch.nb_rows()); |
There was a problem hiding this comment.
Should we check batches content as well?
There was a problem hiding this comment.
Yes I think that should be checked too.
JohanMabille
left a comment
JohanMabille
left a comment
There was a problem hiding this comment.
I agree that the README could be updated with an exmaple of deserializer.
| } | ||
| } | ||
| ``` | ||
|
|
There was a problem hiding this comment.
I think we should still add this to examples file to ensure that the same code (as long as it's copied correctly) is running without failures (no typos etc).
4 participants
No description provided.