Make rounder work with deque, Counter, defaultdict, UserDict and UserList about rounder

[salabim]

The following code is a bit more generic, and better IMHO (not fully tested by me, though).

Be sure to import collections.abc as well!

    def convert(obj):
        if isinstance(obj, complex):
            # this has to be checked before the Number check,
            # as complex is a Number
            return convert(obj.real) + convert(obj.imag) * 1j
        if isinstance(obj, numbers.Number):
            return func(obj, *digits)
        if isinstance(obj, list):
            obj[:] = list(map(convert, obj))
            return obj
        if isinstance(obj, tuple):
            if hasattr(obj, "_fields"):  # it's a namedtuple
                return obj._replace(**convert(obj._asdict()))
            else:
                return tuple(convert(list(obj)))
        if isinstance(obj, set):
            return set(convert(list(obj)))
        if isinstance(obj, frozenset):
            return frozenset(convert(list(obj)))
        if isinstance(obj, collections.abc.Mapping):
            for k, v in obj.items():
                obj[k] = convert(obj[k])
            return obj
        if isinstance(obj, array.array):
            obj[:] = array.array(obj.typecode, convert(obj.tolist()))
            return obj
        if isinstance(obj, collections.deque):
            for i, elem in enumerate(obj):
                obj[i] = convert(elem)
            return obj
        if hasattr(obj, "__dict__"):
            # this should be at the end as some of the above types
            # have a __dict__ attribute
            convert(obj.__dict__)
            return obj

        return obj

[nyggus]

I think we should not enable collections.Counter to be fed into rounder functions. Or rather, if such an object is fed into a rounder function, it should be returned untouched. This is for the following reason: Counter values are integers, so the only functions that will work is signif_object and map_object. And indeed, they do work - but do we want to change Counter values? No! Because then they will be incorrect, as they will not show what Counter aims to show: the number of occurrences of particular values. If a user wants to change these values, the user can (and definitely should) change the Counter instance to, for instance, a dict, and then he or she can use rounder in whatever way he or she wants.

Hence, my suggestion is to add the following if-statement :

if isinstance(obj, frozenset):
    return frozenset(convert(list(obj)))
if isinstance(obj, collections.Counter):
    return obj
if isinstance(obj, collections.abc.Mapping):
    for k, v in obj.items():
        obj[k] = convert(obj[k])
    return obj

The new statement is in the middle. It must be before collections.abc.Mappings, but the later the better for performance, as we do not expect the users to feed Counter instances to rounder functions all that often.

[salabim]

Sorry, but I think I disagree with that as Counter values are not restricted to integers. From the documentation: - The Counter <https://docs.python.org/3/library/collections.html#collections.Counter> class itself is a dictionary subclass with no restrictions on its keys and values. The values are intended to be numbers representing counts, but you *could* store anything in the value field. So, why make an exception for Counter? And also: rounding, ceiling and flooring does not affect integers anyway. There will be more cases that performing the operation in a complex structure (particularly in class instances) is not intended, so the user has to be careful anyway.

…

On Tue, 19 Apr 2022 at 09:44, nyggus ***@***.***> wrote: I think we should not enable collections.Counter to be fed into rounder functions. Or rather, if such an object is fed into a rounder function, it should be returned untouched. This is for the following reason: Counter values are integers, so the only functions that will work is signif_object and map_object. And indeed, they do work - but do we want to change Counter values? No! Because then they will be incorrect, as they will *not* show what Counter aims to show: the number of occurrences of particular values. If a user wants to change these values, the user can (and definitely should) change the Counter instance to, for instance, a dict, and then he or she can use rounder in whatever way he or she wants. Hence, my suggestion is to add the following if-statement : if isinstance(obj, frozenset): return frozenset(convert(list(obj)))if isinstance(obj, collections.Counter): return objif isinstance(obj, collections.abc.Mapping): for k, v in obj.items(): obj[k] = convert(obj[k]) return obj The new statement is in the middle. It must be before collections.abc.Mappings, but the later the better for performance, as we do not expect the users to feed Counter instances to rounder functions all that often. — Reply to this email directly, view it on GitHub <#17 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AHHWWC6Y5DFVLGVW3GSADZLVFZP6LANCNFSM5TQF3O6Q> . You are receiving this because you authored the thread.Message ID: ***@***.***>

[nyggus]

I see the point. I think this is reserved for some special and rare situations indeed, but as Python enables that, we should do so indeed. OK, then, so let's enable collections.Counter that way indeed.

[nyggus]

And frankly, I think the main documentation of the Counter class is slightly misleading, as it says:
"A Counter is a dict subclass for counting hashable objects. It is a collection where elements are stored as dictionary keys and their counts are stored as dictionary values. Counts are allowed to be any integer value including zero or negative counts. "

And only when you read notes for the class, you will see this is not true.

[salabim]

Well, the Python documentation is not very consistent. Anyway, any type is accepted as the value.

[nyggus]

Done.