tests: Avoid common Python default parameter gotcha when mutable dict/list:s are used as default parameter values #16726

pull practicalswift wants to merge 2 commits into bitcoin:master from practicalswift:python-mutable-default-parameter-values changing 3 files +58 −2
  1. practicalswift commented at 10:57 AM on August 26, 2019: contributor

    Avoid common Python default parameter gotcha when mutable dict/list:s are used as default parameter values.

    Examples of this gotcha caught during review:

    Perhaps surprisingly this is how mutable list and dictionary default parameter values behave in Python:

    >>> def f(i, j=[], k={}):
    ...     j.append(i)
    ...     k[i] = True
    ...     return j, k
    ...
    >>> f(1)
    ([1], {1: True})
    >>> f(1)
    ([1, 1], {1: True})
    >>> f(2)
    ([1, 1, 2], {1: True, 2: True})
    

    In contrast to:

    >>> def f(i, j=None, k=None):
    ...     if j is None:
    ...         j = []
    ...     if k is None:
    ...         k = {}
    ...     j.append(i)
    ...     k[i] = True
    ...     return j, k
    ...
    >>> f(1)
    ([1], {1: True})
    >>> f(1)
    ([1], {1: True})
    >>> f(2)
    ([2], {2: True})
    

    The latter is typically the intended behaviour.

    This PR fixes two instances of this and adds a check guarding against this gotcha going forward :-)

  2. Avoid using mutable default parameter values 25dd867150
  3. lint: Catch use of [] or {} as default parameter values in Python functions e4f4ea47eb
  4. fanquake added the label Tests on Aug 26, 2019
  5. Sjors commented at 4:41 PM on August 27, 2019: member

    Oh Python... ACK e4f4ea47ebf7774fb6f445adde7bf7ea71fa05a1. Testing tip: swap the two commits.

  6. fanquake requested review from MarcoFalke on Aug 28, 2019
  7. in test/functional/test_framework/messages.py:807 in e4f4ea47eb
     802 | @@ -803,7 +803,9 @@ def get_siphash_keys(self):
     803 |          return [ key0, key1 ]
     804 |  
     805 |      # Version 2 compact blocks use wtxid in shortids (rather than txid)
     806 | -    def initialize_from_block(self, block, nonce=0, prefill_list = [0], use_witness = False):
     807 | +    def initialize_from_block(self, block, nonce=0, prefill_list=None, use_witness=False):
     808 | +        if prefill_list is None:
    


    promag commented at 1:21 PM on August 28, 2019:

    nit, prefill_list = prefill_list or [0]? Same below.


    MarcoFalke commented at 1:38 PM on August 28, 2019:

    That makes it impossible to pass an empty list

  8. MarcoFalke referenced this in commit cc40b55da7 on Aug 28, 2019
  9. MarcoFalke merged this on Aug 28, 2019
  10. MarcoFalke closed this on Aug 28, 2019

  11. sidhujag referenced this in commit 20a512ab44 on Aug 28, 2019
  12. jasonbcox referenced this in commit a8201635f8 on Oct 15, 2020
  13. practicalswift deleted the branch on Apr 10, 2021
  14. DrahtBot locked this on Aug 18, 2022

github-metadata-mirror

This is a metadata mirror of the GitHub repository bitcoin/bitcoin. This site is not affiliated with GitHub. Content is generated from a GitHub metadata backup.
generated: 2026-04-16 15:14 UTC

This site is hosted by @0xB10C
More mirrored repositories can be found on mirror.b10c.me