Skip to content

TypeError. Edge Case in __init__ of BoxModel and DeckOfCards #93

@bdatko

Description

@bdatko

🐛 Description

When initializing a BoxModel or a DeckOfCards with a size=1 and replace=False, you will receive a

TypeError: '>' not supported between instances of 'NoneType' and 'int' at line 153 within probability_space.py:

if not self.replace and self.size > len(self.box):

The simplest example is drawing a single card without replacement:

>>> DeckOfCards(size=1).draw()

.../lib/python3.10/site-packages/symbulate/probability_space.py:153, in BoxModel.__init__(self, box, size, replace, probs, order_matters)
    149 self.infinite_output_type = InfiniteVector
    151 # If drawing without replacement, check that the number
    152 # of draws does not exceed the number of tickets in the box.
--> 153 if not self.replace and self.size > len(self.box):
    154     raise Exception(
    155         "Cannot draw more tickets (without replacement) "
    156         "than there are tickets in the box."
    157     )

TypeError: '>' not supported between instances of 'NoneType' and 'int'

Another example is initializing a BoxModel without a size and replace=False

>>> die = [1, 2, 3, 4, 5, 6]
>>> BoxModel(die, replace=False, order_matters=True)

.../lib/python3.10/site-packages/symbulate/probability_space.py:153, in BoxModel.__init__(self, box, size, replace, probs, order_matters)
    149 self.infinite_output_type = InfiniteVector
    151 # If drawing without replacement, check that the number
    152 # of draws does not exceed the number of tickets in the box.
--> 153 if not self.replace and self.size > len(self.box):
    154     raise Exception(
    155         "Cannot draw more tickets (without replacement) "
    156         "than there are tickets in the box."
    157     )

TypeError: '>' not supported between instances of 'NoneType' and 'int'

This is due to the ternary operator for self.size in the BoxModel's __init__:

self.size = None if size == 1 else size

I think this bug was first introduced at commit 43e7dad

🤷 Proposal

To my eyes, None has always been the default for size within BoxModel and DeckOfCards because size is matching the interface of numpy.random.choice, which also uses None as the default for size (?)

So I think the safest suggestion would be to move the ternary after the raise Exception within the BoxModel's __init__ function and set the default to size: int = 1 (using the type hint for clarity) as the minimum viable change. This way we can minimize the impact of modifying BoxModel by keeping the logic for the draw method the same.

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions