2
\$\begingroup\$

This code checks the type, length and depth (highest number of nested containers) of the container as well as the type of its sub-containers and its elements. There are also two simple methods for type checking non-nested list / tuples. I mainly want a code review on readability and the quality of the docstrings.

from inspect import isclass
from itertools import islice
from typing import Any, Optional, Union
from collections.abc import Iterable, Sized, Container as GenericContainer
from typing import List, Tuple, Callable


def is_n_container(
        obj: Any,
        matching_type: Union[type, List[type], Tuple[type, ...], None] = None,
        container_type: Union[type, List[type], Tuple[type, ...], None] = None,
        length: Union[int, Callable[[Any], bool], None] = None,
        depth: Union[int, Callable[[Any], bool], None] = None
) -> Optional[bool]:
    """Checks if `obj` is a container with expected properties.

    Verifies that container `obj` and its sub-containers are of a
    type specified in `container_type` and all non-container elements
    of `obj` are of a type specified in `matching_type`.
    The length of the container `obj` and its maximum depth
    is confirmed. More complex requirements can be satisfied
    by providing a function for its length and depth (see notes).

    `None` acts as a "wildcard" for the parameters if the type
    of non-container elements, the type of the container `obj` and
    its sub-containers, the length of container `obj` or
    the depth of container `obj` doesn't matter.

    Parameters
    ----------
    obj : any
        Object that is checked to be a container
        with specified properties.
    matching_type : type or list/tuple of types, optional
        Allowed types for non-container elements of container 'obj'.
    container_type : type or list/tuple of types, optional
        Allowed types for container 'obj' and its sub-containers.
    length : int or function, optional
        Number of elements of depth zero in container 'obj'.
    depth : int or function, optional
        Maximum depth of nested containers.

    Returns
    -------
    bool, optional
        Returns 'None' if a parameter has an incorrect format,
        'True' if 'obj' is container of parameter-defined properties
        or 'False' otherwise.

    Raises
    ------
    ValueError
        If 'length' or 'depth' is smaller than zero.
    TypeError
        If 'length' is specified but 'obj' has no length attribute
        accessible through 'len()' or by iterating over it.

    Warnings
    --------
    **Warning**: If a container type is included in the `matching_type`
    parameter, checking will stop once the container type is found
    without analysing its internal structure.

    **Warning**: If iterating over the container changes it as a side
    effect, the behaviour of `is_n_container` becomes undefined.

    Notes
    -----
    A function used as argument for parameter `length` or
    parameter `depth` should take an object (`obj`) as argument and
    return `True` if the object has the required length/depth or
    `False` otherwise.

    Variable length containers also include empty containers.
    If empty containers should be excluded, check
    for empty containers separately or use
    `not is_n_container(obj, length=0, **kwargs) and
    is_n_container(obj, length=None, **kwargs)`.

    Even when a sub-container is empty it adds another level of depth
    (e.g. ([], 1, 2, 3) has depth 1), unless the sub-container is a
    type in parameter `matching_type` (e.g. with
    `matching_type=(int, list)` ([], 1, 2, 3) has depth 0).

    Examples
    --------
    Types of elements: int, float, str \n
    Types of containers: list, tuple \n
    Depth 0: [(I), 2, 3, (II), (III), 2.3] - Length: 6 \n
    Depth 1: I - 'a', 1, 'b' | II - 4, 5 | III - (IV), 9 \n
    Depth 2: IV - (V), 8 \n
    Depth 3: V - 6, 7


    >>> container = [['a', 1, 'b'], 2, 3, (4, 5), (((6, 7), 8), 9), 2.3]

    Use all parameters and `bool` as an unused `matching_type` parameter

    >>> is_n_container(container, matching_type=(int, str, float, bool),
    ...                container_type=(tuple, list), length=6, depth=3)
    True

    Wrong length

    >>> is_n_container(container, matching_type=(int, str, float, bool),
    ...                container_type=(tuple, list), length=5, depth=3)
    False

    Allow more depth than needed

    >>> is_n_container(container, matching_type=(int, str, float),
    ...                container_type=(tuple, list), depth=5)
    True

    Expect a too shallow structure

    >>> is_n_container(container, matching_type=(int, str, float),
    ...                container_type=(tuple, list), length=6, depth=2)
    False

    Missing `matching_type` arguemnt `float`

    >>> is_n_container(container, matching_type=(int, str),
    ...                container_type=(tuple, list))
    False

    Missing 'container_type' argument 'list'

    >>> is_n_container(tuple(container),
    ...                matching_type=(int, str, float),
    ...                container_type=tuple)
    False

    Container `obj` is still a list

    >>> is_n_container(container[1:-1], matching_type=int,
    ...                container_type=tuple)
    False

    Now all containers are of type `tuple`

    >>> is_n_container(tuple(container[1:-1]), matching_type=int,
    ...                container_type=tuple)
    True

    Using `None` for an argument allows for a wide variety of
    acceptable containers, but not non-container objects

    >>> containers = [[1, 2, 3], ('a', 'b', 'c'), {1.5, 2.4, 3.3}, True]
    >>> for i in (0, 1, 2, 3):
    ...     is_n_container(containers[i], depth=1)
    True
    True
    True
    False

    Even if the container is empty it adds another level of depth

    >>> container = [(), ([], 1), 2]
    >>> for i in (1, 2):
    ...     is_n_container(container, matching_type=int, depth=i)
    False
    True

    Unless the container is a type in parameter `matching_type`

    >>> is_n_container(container, matching_type=(int, list), depth=1)
    True

    """
    if _check_parameters(matching_type, container_type, length, depth):
        _matching_type = (matching_type if not isinstance(matching_type, list)
                          else tuple(matching_type))
        _container_type = (container_type
                           if not isinstance(container_type, list)
                           else tuple(container_type))
        if (not isinstance(obj, GenericContainer)
                or not (_container_type is None
                        or isinstance(obj, _container_type))
                or (isinstance(length, Callable) and not length(obj))
                or (isinstance(depth, Callable) and not depth(obj))):
            return False
        _length = length if isinstance(length, int) else None
        _depth = depth if isinstance(depth, int) else float('inf')
        if _length is not None:
            if _length < 0:
                raise ValueError("length of the container must be positive," +
                                 f" not {_length}")
            if not (isinstance(obj, Sized) or isinstance(obj, Iterable)
                    or hasattr(obj, '__getitem__')):
                raise TypeError(f"object of type '{type(obj).__name__}'" +
                                " has no attribute len()")
            if ((isinstance(obj, Sized) and _length != len(obj)) or (
                    (isinstance(obj, Iterable) or hasattr(obj, '__getitem__'))
                    and _length != sum(1 for _ in islice(obj, _length + 1)))):
                return False
        if _depth < 0:
            raise ValueError("depth of the container must be positive," +
                             f" not {_depth}")
        if (matching_type is None or not (isinstance(obj, Iterable)
                                          or hasattr(obj, '__getitem__'))):
            return (_container_type is None
                    or isinstance(obj, _container_type))
        return _is_container(obj, _matching_type, _container_type, _depth)
    return None


def _check_parameters(matching_type, container_type, length, depth) -> bool:
    """Returns `True` if all arguments for `is_n_container`
    have the correct format."""
    return (True  # Added for better readability
            and (matching_type is None
                 or isinstance(matching_type, type)
                 or is_n_list(matching_type, None, type)
                 or is_n_tuple(matching_type, None, type))
            and (container_type is None
                 or (isclass(container_type)
                     and issubclass(container_type, GenericContainer))
                 or (isinstance(container_type, list)
                     and all((isclass(c_type)
                              and issubclass(c_type, GenericContainer))
                             for c_type in container_type))
                 or (isinstance(container_type, tuple)
                     and all((isclass(c_type)
                              and issubclass(c_type, GenericContainer))
                             for c_type in container_type)))
            and (length is None
                 or isinstance(length, int)
                 or isinstance(length, Callable))
            and (depth is None
                 or isinstance(depth, int)
                 or isinstance(depth, Callable)))


def _is_container(obj: Iterable,
                  matching_type: Union[type, Tuple[type, ...]],
                  container_type: Union[type, Tuple[type, ...], None],
                  depth: Union[int, float]) -> bool:
    """Checks properties of container `obj` recursively.
    Used in `is_n_container`."""
    return depth >= 0 and all(
        isinstance(element, matching_type)
        or ((isinstance(element, Iterable) or hasattr(element, '__getitem__'))
            and (container_type is None or isinstance(element, container_type))
            and _is_container(element, matching_type, container_type, depth-1))
        for element in obj
    )


def is_n_list(obj: Any, length: Optional[int],
              matching_type: Union[type, Tuple[type, ...]]) -> bool:
    """Checks if `obj` is a list with expected properties.

    Parameters
    ----------
    obj : any
        Object that is checked to be a list
        with specified properties.
    matching_type : type or tuple of types, optional
        Allowed types for non-container elements of container 'obj'.
    length : int, optional
        Number of elements in container 'obj'.

    Returns
    -------
    bool
        Returns 'True' if 'obj' is a list and all its elements are
        of a type defined in 'matching_type'.

    See Also
    --------
    `is_n_container`: Checks if `obj` is a container with
    expected properties.

    """
    return (isinstance(obj, list) and (length is None or length == len(obj))
            and all(isinstance(element, matching_type) for element in obj))


def is_n_tuple(obj: Any, length: Optional[int],
               matching_type: Union[type, Tuple[type, ...]]) -> bool:
    """Checks if `obj` is a tuple with expected properties.

    Parameters
    ----------
    obj : any
        Object that is checked to be a tuple
        with specified properties.
    matching_type : type or tuple of types, optional
        Allowed types for non-container elements of container 'obj'.
    length : int, optional
        Number of elements in container 'obj'.

    Returns
    -------
    bool
        Returns 'True' if 'obj' is a tuple and all its elements are
        of a type defined in 'matching_type'.

    See Also
    --------
    `is_n_container`: Checks if `obj` is a container with
    expected properties.

    """
    return (isinstance(obj, tuple) and (length is None or length == len(obj))
            and all(isinstance(element, matching_type) for element in obj))

Additional specific questions:

  1. Should I scrap the whole thing from a Python standpoint, since its follows more of a LBYL principle than a EAFP principle (LBYL/EAFP)?
  2. If not the whole thing, should I get rid of the _check_parameters method?
  3. Are there enough / too many examples for is_n_container in the docstring and are these examples helpful or should they be put in a example context?
  4. PEP8 suggests to use blank lines in functions sparingly. Should I use some in is_n_container? And if yes, where?
  5. Is the iteration warning about a possible changed container necessary / helpful?
  6. Is it ok to use three different indentations for the function parameters (*see remarks)?

Some remarks:

I use python 3.8.8 and pycharm 2020.3.4 . Since the sections 'Parameters', 'Returns' and 'Raises' won't format single back-ticks (``) in the docstring correctly (for me), I used single quotation marks ('') instead. I imported List, Tuple and Callable from typing seperatly, because in python 3.9 List and Tuple should be replaced with the generic list and tuple and Callable should be imported from collections.abc . I don't use any type hinting in _check_parameters as they all have type Any (i want to accept any type and return if they are of the desired type).

*I use three different indentations because I usually use:

  1. If I can get it into two lines

    def func(var1: type, var2: type,
             var3: type, var4: type) -> type
    
  2. Else if I can get the type hint of the result into the line with the last variable

    def name_too_long_to_get_parameters_in_two_lines(long_var_name1: type,
                                                     long_var_name2: type,
                                                     long_var_name3: type) -> type
    
  3. Else

    def name_and_result_type_hinting_too_long(
        var1: type,
        var2: type,
        var3: type
    ) -> really_long_result_type
    
\$\endgroup\$

1 Answer 1

0
\$\begingroup\$

Assuming that you decide to retain _check_parameters(), here's an illustration of some alternative techniques for organizing a complex boolean check. (1) The isinstance() function will take a tuple of types, so you can do some consolidation. (2) If you need to check for None and check for types, you can do it all in one shot. (3) Some of your checks were repetitive; help the reader by factoring things out. (4) Organize the checks like a pretty-printed data structure because it gets the parens/brackets working for you to convey logical structure. (5) Sometimes simple and fairly banal code comments can function as visual/organizational sign posts to guide the reader. (6) I prefer the lines of code to lead with the substance rather than the boolean operator -- which is mostly a stylistic preference, but I do think it combines better in these kinds of complex checks. For example, I felt no readability-driven urge to add a preliminary True and to the expression. Also, most people use editors with syntax highlighting, so the operators pop out visually and there's no need to waste the prime real estate (the start of each line) on the operator.

TNone = type(None)

check_ctype_seq = lambda ctypes, cls: (
    isinstance(ctypes, cls) and
    all(
        isclass(ct) and
        issubclass(ct, GenericContainer)
        for ct in ctypes
    )
)

return (
    # Length and depth.
    isinstance(length, (int, Callable, TNone)) and
    isinstance(depth, (int, Callable, TNone)) and
    # Matching type.
    (
        isinstance(matching_type, (TNone, type)) or
        is_n_list(matching_type, None, type) or
        is_n_tuple(matching_type, None, type)
    ) and
    # Container_type.
    (
        container_type is None or
        (
            isclass(container_type)
            and issubclass(container_type, GenericContainer)
        ) or
        check_ctype_seq(container_type, list) or
        check_ctype_seq(container_type, tuple)
    )
)
\$\endgroup\$
5
  • \$\begingroup\$ (1) & (2): In the beginning I did check it like you suggested using a type(None) and a single isinstance(), but then decided against it after reading the comments here, here and here. And that automatically translated in my mind into splitting the rest too. \$\endgroup\$
    – user239216
    Commented Mar 20, 2021 at 20:41
  • \$\begingroup\$ (3) & (6): I did factor the repetitive checks out, but decided against it again after thinking it would hinder readability to scroll to a different function for such a simple check. For the boolean operator I simply hit the line limit with one expression and then decided to put it at the beginning of the line for consistencies sake. Besides changing that I definitely implement (4) & (5). Thanks! \$\endgroup\$
    – user239216
    Commented Mar 20, 2021 at 20:49
  • \$\begingroup\$ @Jack Perhaps I'm overlooking some detail, but I don't read any of those links as ruling out consolidating isinstance() checks or including type(None) among those checks. Regarding the repeated code, you really do want to factor that out -- either to a nearby lambda (as I did) or to a separate function. It's beyond the threshold that most developers I work with would tolerate for repetition -- partly because of the nature of the check itself (it needs 2 tests, one requiring a comprehension) and especially because it's being used within an already-complex context. \$\endgroup\$
    – FMc
    Commented Mar 20, 2021 at 21:08
  • \$\begingroup\$ They didn't rule it out, but some of the comment suggested using var is None or isinstance(var, otherTypes) instead of isinstance(var, (TNone, otherTypes)). \$\endgroup\$
    – user239216
    Commented Mar 20, 2021 at 21:37
  • \$\begingroup\$ @Jack I saw people asserting that but they only gave one reason I found convincing: in cases where you must heavily optimize for speed. As always context matters: I would never use instance() to test a value for None in an isolated, simple context; but in a very complex context, where you're already using instance() to check for other types, you might as well be practical and prioritize code weight and therefore readability. But people do evaluate such considerations differently -- which is one reason why writing software is closer to writing than it is to more rule-bound activities. Good luck \$\endgroup\$
    – FMc
    Commented Mar 20, 2021 at 21:59

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.