Modify getsize to return total size, not just the top level #1815
Conversation
zarr/storage.py
Outdated
| for child in scandir(fs_path): | ||
| if child.is_file(): | ||
| size += child.stat().st_size | ||
| for root, dirs, files in os.walk(fs_path): |
There was a problem hiding this comment.
Should also include the size of directories - these can be significant when there's a large number of files within a directory. The result here should be the same as du, which does take this into account.
benjeffery
force-pushed
the
recursive-getsize
branch
from
85623b7 to
c5d4a5e
Compare
|
I guess it's worth asking what the fate of this will be for the v3 branch - if the code is just in as a short-lived bugfix that dies when we switch over the v3 then I'm not sure it's worth pushing through for our purposes @benjeffery |
Yes, I'm not sure on the time scales here, so would be good to know. v3 behaviour is to report the total size as in this PR. I'm missing some tests here that I didn't spot locally as they were skipped for optional deps, happy to patch those up if this looks like it's needed. |
|
👋 thanks for the PR, and sorry it's been left to wilt a bit! I've just run into this too at #2174, so I would be keen to see this PR finished. If you can patch up the remaining test failures, and rebase on to the current |
benjeffery
force-pushed
the
recursive-getsize
branch
3 times, most recently
from
b884714 to
0bee609
Compare
benjeffery
force-pushed
the
recursive-getsize
branch
from
0bee609 to
86d737f
Compare
|
I've changed the base branch from |
|
Hmmm I had a look at fixing the tests, and it looks non-trivial... |
Yes, I also had a go and the current tests really aren't set up for the directory size to be different across store types. |
Fixes #253
TODO: