Fix calculating datasets stats size #418
Conversation
dreadatour
requested review from
shcheklein,
ilongin,
skshetry,
mattseddon and
a team
| """ | ||
| Returns tuple with dataset stats: total number of rows and total dataset size. | ||
| """ | ||
| dataset = self.get_dataset(name) | ||
| dataset_version = dataset.get_version(version) | ||
| dataset_version = dataset.get_version(version or dataset.latest_version) |
There was a problem hiding this comment.
datachain dataset-stats ds-name command now takes latest dataset version if version is not provided instead of raising an error.
| query = select(*expressions) | ||
| ((nrows, *rest),) = self.db.execute(query) | ||
| return nrows, rest[0] if rest else None | ||
| return nrows, sum(rest) if rest else 0 |
There was a problem hiding this comment.
We are waiting for int in many places, errors happens if numbers of rows is set, but size is None (for example, here). It is better to return 0 in such cases.
There was a problem hiding this comment.
Error looks like this:
$ datachain dataset-stats laion_pq
Number of objects: 2
Error: bad operand type for abs(): 'NoneType'
$
dreadatour
force-pushed
the
fix-calculating-stats-size
branch
from
af99a34 to
bbb129b
Compare
Deploying datachain-documentation with
|
| Latest commit: |
2c8be93
|
| Status: | ✅ Deploy successful! |
| Preview URL: | https://8638fafa.datachain-documentation.pages.dev |
| Branch Preview URL: | https://fix-calculating-stats-size.datachain-documentation.pages.dev |
dreadatour
force-pushed
the
fix-calculating-stats-size
branch
from
bbb129b to
2c8be93
Compare
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #418 +/- ##
==========================================
- Coverage 87.09% 87.09% -0.01%
==========================================
Files 92 92
Lines 9929 9928 -1
Branches 2032 2032
==========================================
- Hits 8648 8647 -1
Misses 927 927
Partials 354 354
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. |
It looks like calculating datasets stats is broken in some cases.
The way it works now is we are calculating the sum of either
file__sizeorsizefields in all dataset rows. I thinksizeis not used anymore andfile__sizewill works only for listings.In this PR I am calculating
sizedataset stat as sum of all fields with name ending withfile__size. This way we will be able to calculate listing size (file__sizefield) and other signals, likelaion.file.size,source.file.size,emd.file.size, etc. Also we are now calculating sum of allfile.sizefields instead of calculating only one of them.