4
\$\begingroup\$

I am trying to write a script that will traverse through my directory and sub directory and list the number of files in a specific size.

For example, 0kb-1kb: 3, 1kb-4kb:4, 4-16KB: 4, 16kb-64-kb:11. This goes on in multiples of 4.

I am able to get the list of file numbers, the size in human readable format and find the number of files in a size group. But I feel my code is very messy and not anywhere near the standard.

import os
suffixes = ['B', 'KB', 'MB', 'GB', 'TB', 'PB']
route = raw_input('Enter a location')
def human_Readable(nbytes):
    if nbytes == 0: return '0 B'
    i = 0
    while nbytes >= 1024 and i < len(suffixes)-1:
        nbytes /= 1024.
        i += 1
    f = ('%.2f' % nbytes).rstrip('0').rstrip('.')
    return '%s %s' % (f, suffixes[i])
def file_Dist(path, start,end):
    counter = 0
    for path, subdir, files in os.walk(path):
        for r in files:
            if os.path.getsize(os.path.join(path,r)) > start and os.path.getsize(os.path.join(path,r)) < end:
                counter += 1
        print "Number of files greater than %s less than %s:" %(human_Readable(start), human_Readable(end)),  counter
file_Dist(route, 0, 1024)
file_Dist(route,1024,4095)
file_Dist(route, 4096, 16383)
file_Dist(route, 16384, 65535)
file_Dist(route, 65536, 262143)
file_Dist(route, 262144, 1048576)
file_Dist(route, 1048577, 4194304)
file_Dist(route, 4194305, 16777216)
\$\endgroup\$
3
\$\begingroup\$
  • suffixes = ['B', 'KB', 'MB', 'GB', 'TB', 'PB'] you never change the value of suffixes, so it should be a constant, the standard naming of constant in Python is ALL_UPPERCASE, rename it to SUFFIXES

  • You should name variables with snake_case.

  • You wrote for r in files: the details count: the standard one-letter identifier for file is f, use that.

  • Look at f = ('%.2f' % nbytes).rstrip('0').rstrip('.'), f is a meaningless one-letter name. You should use a longer and more meaningful name

  • You are checking and i < len(suffixes)-1 in the loop: it is not possible that the user has a file bigger than one Petabyte (10**15 bytes) so you can safely remove that check. and it is very good that you that, code that crashes when you find an unexpected condition should be avoided with error checking like this one. (Thanks to @Veedrac for correcting me)


f = ('%.2f' % nbytes).rstrip('0').rstrip('.')
return '%s %s' % (f, suffixes[i])

You are a C programmer, aren't you? :) Using "%s" string formatting in Python is considered obsolete and should be avoided, instead use string.format()


print "Number of files greater than %s less than %s:" %(human_Readable(start), human_Readable(end)),  counter

You are printing from inside a function this is not nice, you should return values, not print them.


It is common to take the parametres from the command line so:

route = raw_input('Enter a location')

should become:

import sys
route = sys.argv[1]

this also makes the code compatible with Python 3 if you add brackets around the print


You are repeating yourself so much:

file_Dist(route, 0, 1024)
file_Dist(route,1024,4095)
file_Dist(route, 4096, 16383)
file_Dist(route, 16384, 65535)
file_Dist(route, 65536, 262143)
file_Dist(route, 262144, 1048576)
file_Dist(route, 1048577, 4194304)
file_Dist(route, 4194305, 16777216)

It would be better to use a loop:

for start in [1024,4096,16384,262144,1048577,4194305]:
    end = start * 4
    file_Dist(route,start,end)

You may want to import this script, using if __name__ will allow you to run the tests only if this script is the main file

def test():
    for start in [1024,4096,16384,262144,1048577,4194305]:
        end = start * 4
        file_Dist(route,start,end)

if __name__ == "__main__":
    test()
\$\endgroup\$
  • \$\begingroup\$ I disagree about removing the i < len(suffixes)-1 check; it may be "impossible" for a file to be that large but that doesn't mean that a bunch of files can't be that large or that you'll never pass anything else into the function. \$\endgroup\$ – Veedrac Jan 9 '15 at 10:15
  • \$\begingroup\$ If you are concerned about it you can add EX and ZB, now nothing at all will be bigger than a Zettabyte, (10**21) bytes. \$\endgroup\$ – Caridorc Jan 9 '15 at 12:52
  • 1
    \$\begingroup\$ It's not even that; what if the OS freaks out and reports (uint64_t)-1 bytes? Maybe it's a networked drive. Maybe you calculated some physics and wanted a routine to print it out. Maybe you get user input on PermissionErrors and don't want your printing routine to crash when the user messes around. Your "I don't need to check for out of bounds because the numbers won't be that big" sounds a lot like "why would I need error-checking if I don't write bugs?" \$\endgroup\$ – Veedrac Jan 9 '15 at 13:13
1
\$\begingroup\$

Symlinks

Your code will double-count for symlinks and fail on broken symlinks. One way to avoid this is with

os.lstat(...).st_size

Props to asmeurer.

Your condition then becomes:

os.lstat(os.path.join(path,r)).st_size > start and os.lstat(os.path.join(path,r)).st_size < end

Technically this is also prone to race conditions; consider something that is 100gb and get replaced with a blank file - at no point should it be counted yet both checks could succeed. In this case it's not a big deal since there is a much greater concern of paths changing since you checked for then with os.walk.

Style and organization

On a brighter note, you can use chained comparisons to make this

start < os.lstat(os.path.join(path,r)).st_size < end

I see you do

for path, subdir, files in os.walk(path):

Repurposing a variable like this is bad style; try for subpath, ... in os.walk(path) instead.

If you move the os.path.join and looping to a separate function

def visit_all(path):
    for subpath, subdir, files in os.walk(path):
        for file in files:
            yield os.path.join(subpath, file)

You can do just

count = sum(start < os.lstat(subpath).st_size < end for subpath in visit_all(path))

Using binning for counting

However, you do

file_Dist(route, 0, 1024)
file_Dist(route, 1024,4095)
file_Dist(route, 4096, 16383)
file_Dist(route, 16384, 65535)
file_Dist(route, 65536, 262143)
file_Dist(route, 262144, 1048576)
file_Dist(route, 1048577, 4194304)
file_Dist(route, 4194305, 16777216)

suggesting that what you want are bins of that spacing. This would let you do

file_dists(route, [1024, 4095, 16383, 65535, 262143, 1048576, 4194304, 16777216])

to calculate all of these at once in a single pass.

note: As you have written it, something with a size of exactly 1024 bytes will not be counted in any pass, and the same for the other named sizes. My variant will not have this property.

You should create bins to count the values:

# One for each range (size[i-1], size[i]] and another for (size[-1], +∞]
bins = [0] * (len(sizes) + 1)

And for each value find highest the position in the input sizes that is no greater than the size:

for file in visit_all(path):
    size = os.lstat(file).st_size
    insert_at = bisect_left(sizes, size)

Which is where you add the count:

    bins[insert_at] += 1

bisect_left is from the bisect module.

You should return this result and the caller can print the values with

for lo, hi, count in zip(sizes, sizes[1:], bincounts):
    print("Number of files greater than {} less than {}: {}".format(
        human_Readable(lo),
        human_Readable(hi),
        count
    ))

The last range ((size[-1], +∞]) is not printed here.

Sticking this together, we get:

import os
from bisect import bisect_left

suffixes = ['B', 'KB', 'MB', 'GB', 'TB', 'PB']
route = raw_input('Enter a location')
def human_Readable(nbytes):
    if nbytes == 0: return '0 B'
    i = 0
    while nbytes >= 1024 and i < len(suffixes)-1:
        nbytes /= 1024.
        i += 1
    f = ('%.2f' % nbytes).rstrip('0').rstrip('.')
    return '%s %s' % (f, suffixes[i])

def visit_all(path):
    for subpath, subdir, files in os.walk(path):
        for file in files:
            yield os.path.join(subpath, file)

def file_dists(path, sizes):
    # One for each range (size[i-1], size[i]] and another for (size[-1], +∞]
    bins = [0] * (len(sizes) + 1)

    for file in visit_all(path):
        size = os.lstat(file).st_size
        insert_at = bisect_left(sizes, size)

        bins[insert_at] += 1

    return bins

sizes = 1024, 4095, 16383, 65535, 262143, 1048576, 4194304, 16777216
bincounts = file_dists(route, sizes)

for lo, hi, count in zip(sizes, sizes[1:], bincounts):
    print("Number of files greater than {} less than {}: {}".format(
        human_readable(lo),
        human_readable(hi),
        count
    ))

Calculating human-readable names

The function, as with the others, should be snake_case, so human_readable. I shall call it this henceforth.

Getting the route has nothing to do with this function; it should be moved to the relevant place.

Your

f = ('%.2f' % nbytes).rstrip('0').rstrip('.')

is rather fragile, although it will work. Better than string manipulations is working directly on a decimal representation:

f = Decimal(15.999023).quantize(Decimal('0.00')).normalize()

If you move to Python 3 this can just be

f = round(Decimal(15.9990234375), 2).normalize()

Because of this change, the function now works for 0 correctly so you can remove the explicit check!

Instead of using i in the loop you can just do

for suffix in suffixes:
    if nbytes < 1024:
        break
    nbytes /= 1024.

and then

return '{} {}'.format(rounded, suffix)

Finally, I would make suffixes a tuple for the immutability and make it UPPERCASE to follow PEP 8.

This gives

suffixes = 'B', 'KB', 'MB', 'GB', 'TB', 'PB'
def human_readable(nbytes):
    for suffix in suffixes:
        if nbytes < 1024:
            break
        nbytes /= 1024.

    rounded = Decimal(nbytes).quantize(Decimal('0.00')).normalize()
    return '{} {}'.format(rounded, suffix)

Finishing up

Put everything else in main. Add error checking to ignore inaccessible files. Alias input to raw_input for Python 3 compatibility.

I would also consider if you really want your given numbers; in binary they are

              10000000000
             111111111111
           11111111111111
         1111111111111111
       111111111111111111
    100000000000000000000
  10000000000000000000000
1000000000000000000000000

It would make more sense to have

              10000000000
            1000000000000
          100000000000000
        10000000000000000
      1000000000000000000
    100000000000000000000
  10000000000000000000000
1000000000000000000000000

which can be easily generated with

[2**i for i in range(10, 26, 2)]

Once you're done, you get

# encoding: utf-8

from __future__ import print_function

import os

from bisect import bisect_left
from decimal import Decimal

try:
    input = raw_input
except NameError:
    pass

SUFFIXES = 'B', 'KB', 'MB', 'GB', 'TB', 'PB'
def human_readable(nbytes):
    for suffix in SUFFIXES:
        if nbytes < 1024:
            break
        nbytes /= 1024.

    rounded = Decimal(nbytes).quantize(Decimal('0.00')).normalize()
    return '{} {}'.format(rounded, suffix)

def visit_all(path, onerror=None):
    for subpath, subdir, files in os.walk(path, onerror=onerror):
        for file in files:
            yield os.path.join(subpath, file)

def file_dists(path, sizes):
    # One for each range (size[i-1], size[i]] and another for (size[-1], +∞]
    bins = [0] * (len(sizes) + 1)

    for file in visit_all(path, onerror=print):
        try:
            size = os.lstat(file).st_size
        except OSError as err:
            print(err)
            continue

        insert_at = bisect_left(sizes, size)
        bins[insert_at] += 1

    return bins

def main():
    route = input('Enter a location: ')

    sizes = [2**i for i in range(10, 26, 2)]
    bincounts = file_dists(route, sizes)

    for lo, hi, count in zip(sizes, sizes[1:], bincounts):
        print("Number of files greater than {} less than {}: {}".format(
            human_readable(lo),
            human_readable(hi),
            count
        ))

if __name__ == "__main__":
    main()
\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.