factory_boy icon indicating copy to clipboard operation
factory_boy copied to clipboard

Reproducible randomness can't be achieved on Python < 3.6

Open x-yuri opened this issue 5 years ago • 3 comments

Description

The code relies on class attributes and dicts being iterated in the declaration/insertion order.

To Reproduce

a.py:

import factory, factory.random

factory.random.reseed_random(53389)

class User:
    def __init__(*args, **kwargs):
        print(kwargs)

class UserFactory(factory.Factory):
    class Meta:
        model = User

    s1 = factory.Faker('name')
    l2 = factory.Faker('locale')

u = UserFactory()
$ docker run --rm -itv a.py:/a.py python:3.5-alpine sh -c 'pip install factory-boy && for i in `seq 1 10`; do python /a.py; done'
...
{'l2': 'is_IS', 's1': 'Rachel Rogers MD'}
{'s1': 'Randy Esparza', 'l2': 'hne_IN'}
{'s1': 'Randy Esparza', 'l2': 'mai_IN'}
{'s1': 'Randy Esparza', 'l2': 'ro_RO'}
{'l2': 'shs_CA', 's1': 'Rachel Rogers MD'}
{'s1': 'Randy Esparza', 'l2': 'nr_ZA'}
{'l2': 'apn_IN', 's1': 'Rachel Rogers MD'}
{'s1': 'Randy Esparza', 'l2': 'om_ET'}
{'s1': 'Randy Esparza', 'l2': 'zu_ZA'}
{'s1': 'Randy Esparza', 'l2': 'zu_ZA'}

$ docker run --rm -itv a.py:/a.py python:3.6-alpine sh -c 'pip install factory-boy && for i in `seq 1 10`; do python /a.py; done'
...
{'s1': 'Randy Esparza', 'l2': 'so_ET'}
{'s1': 'Randy Esparza', 'l2': 'so_ET'}
{'s1': 'Randy Esparza', 'l2': 'so_ET'}
{'s1': 'Randy Esparza', 'l2': 'so_ET'}
{'s1': 'Randy Esparza', 'l2': 'so_ET'}
{'s1': 'Randy Esparza', 'l2': 'so_ET'}
{'s1': 'Randy Esparza', 'l2': 'so_ET'}
{'s1': 'Randy Esparza', 'l2': 'so_ET'}
{'s1': 'Randy Esparza', 'l2': 'so_ET'}
{'s1': 'Randy Esparza', 'l2': 'so_ET'}

Notes

The changing locales are partly cased by faker, since they are declared in a dict.

In 3.6 they decided to change the dict implementation:

The order-preserving aspect of this new implementation is considered an implementation detail and should not be relied upon (this may change in the future, but it is desired to have this new dict implementation in the language for a few releases before changing the language spec to mandate order-preserving semantics for all current and future Python implementations; this also helps preserve backwards-compatibility with older versions of the language where random iteration order is still in effect, e.g. Python 3.5).The order-preserving aspect of this new implementation is considered an implementation detail and should not be relied upon (this may change in the future, but it is desired to have this new dict implementation in the language for a few releases before changing the language spec to mandate order-preserving semantics for all current and future Python implementations; this also helps preserve backwards-compatibility with older versions of the language where random iteration order is still in effect, e.g. Python 3.5).

In 3.7 that became official:

the insertion-order preservation nature of dict objects has been declared to be an official part of the Python language spec

I decided to report the issue since I ran into it, but should it be fixed?.. I'm not sure. It can probably be closed.

x-yuri avatar Jan 08 '21 13:01 x-yuri

Thanks for the report! This is indeed a bug, and special care had been taken to ensure that we didn't fall prey to those issues :/

The reproducible code is very useful, I'll look into it right away!

Note that factory_boy's declarations use a special base class which keeps declaration in the order they were parsed by the Python interpreter. That order should also be used later on in the build steps.

So, weird bug :)

rbarrois avatar Jan 08 '21 14:01 rbarrois

It might be of benefit to know that I opened a related issue in the faker's repo. And they said they no longer support Python 3.5. Which makes factory-boy pursuing the goal less reasonable.

Having that said, a better test code would be (to not involve the faker's bug):

import factory, factory.random

factory.random.reseed_random(53389)

class User:
    def __init__(*args, **kwargs):
        print(kwargs)

class UserFactory(factory.Factory):
    class Meta:
        model = User

    n1 = factory.Faker('name')
    d1 = factory.Faker('random_digit')

u = UserFactory()
$ docker run --rm -itv $PWD/a.py:/a.py python:3.5-alpine sh -c 'pip install factory-boy && for i in `seq 1 10`; do python /a.py; done'
...
{'d1': 5, 'n1': 'Kelly Marquez'}
{'d1': 5, 'n1': 'Kelly Marquez'}
{'d1': 5, 'n1': 'Kelly Marquez'}
{'d1': 5, 'n1': 'Kelly Marquez'}
{'n1': 'Randy Esparza', 'd1': 2}
{'n1': 'Randy Esparza', 'd1': 2}
{'n1': 'Randy Esparza', 'd1': 2}
{'n1': 'Randy Esparza', 'd1': 2}
{'d1': 5, 'n1': 'Kelly Marquez'}
{'n1': 'Randy Esparza', 'd1': 2}

Oh, and I messed up mounting the file into a container: a.py:... -> $PWD/a.py:... (or /tmp/a.py for that matter). I postedited the command, there's no denying it :)

Note that factory_boy's declarations use a special base class which keeps declaration in the order they were parsed by the Python interpreter. That order should also be used later on in the build steps.

Interesting idea, it didn't occur to me. But for some reason the declarations are evaluated in a random order. Unless you do this:

-        for field_name in declarations:
+        for field_name in declarations.sorted():
             self.attributes[field_name] = getattr(self.stub, field_name)

https://github.com/FactoryBoy/factory_boy/blob/3.2.0/factory/builder.py#L198

Then it all works :) Unless you're running factory-boy >= 3.2.0:

https://github.com/FactoryBoy/factory_boy/commit/64f10a18525572a5158b753d8f20195553fa5c5f

x-yuri avatar Jan 09 '21 07:01 x-yuri

FactoryBoy no longer supports Python 3.5 either. You can refer to the Support Policy to see what versions of Python are supported.

francoisfreitag avatar Jan 11 '21 08:01 francoisfreitag