pysize icon indicating copy to clipboard operation
pysize copied to clipboard

Failing iterable check on types

Open rpkilby opened this issue 7 years ago • 7 comments

Hi. The iterable check on Line 25 doesn't work when encountering types. https://github.com/bosswissam/pysize/blob/e94d25c9e240ab359f22ffaea7f99f00b736c24e/pysize.py#L25

I believe this should be changed to

    elif isinstance(obj, collections.Iterable) and not isinstance(obj, (str, bytes, bytearray)):

rpkilby avatar Oct 06 '17 23:10 rpkilby

@rpkilby can you provide an example of the failure?

bosswissam avatar Oct 06 '17 23:10 bosswissam

@rpkilby Hi. I will change this algorithm to use a recursive approach. For now if you are blocked you can use sys.setrecursionlimit. Good catch!

bosswissam avatar Oct 06 '17 23:10 bosswissam

Yep

>>> from collections import OrderedDict
>>> from pysize import get_size
>>> get_size(OrderedDict)
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
  File "/Users/bagel/Documents/django-rest-framework-filters/pysize.py", line 26, in get_size
    size += sum((get_size(i, seen) for i in obj))
TypeError: 'type' object is not iterable

rpkilby avatar Oct 06 '17 23:10 rpkilby

@bosswissam - it's not a recursion issue, just a simple TypeError. I've fixed it locally by making the change in my original comment.

rpkilby avatar Oct 06 '17 23:10 rpkilby

Basically, OrderedDict.__iter__ exists as an unbound method, but the OrderedDict type is not itself iterable, which is why the preceding check erroneously passes.

Hope that helps.

rpkilby avatar Oct 07 '17 00:10 rpkilby

@rpkilby Apologies I seem to have had another issues open and left a comment on this one mistakenly. Yup will make this change, also feel free to send a PR if you'd like.

bosswissam avatar Oct 09 '17 06:10 bosswissam

Please correct me if I'm wrong, but if a TypeError is occurring simply because a type object has been encountered, wouldn't adding type to the isinstance classinfo tuple fix this issue?

elif hasattr(obj, '__iter__') and not isinstance(obj, (type, str, bytes, bytearray)):

kelechifletcher avatar Oct 19 '17 15:10 kelechifletcher