Add pickle support to dynamically created models and generics #1686

Bobronium · 2020-07-05T16:04:19Z

Change Summary

Always set __module__ to created class (try to find the actual one, in case of failure fallback to pydantic.main)
Set reference of created concrete model to it's module to allow pickling (not applied to models created in functions)
~~Add pydantic.utils.ensure_picklable which will allow to make model, created inside a functions or/and referenced with a different name from class name pickable~~

Related issue number

Closes #1667
Closes #1734

Checklist

Unit tests for the changes exist
Tests pass on CI and coverage remains at 100%
Documentation reflects the changes where applicable
changes/<pull request or issue id>-<github username>.md file added describing change
(see changes/README.md for details)

codecov · 2020-07-05T16:08:09Z

Codecov Report

Merging #1686 into master will decrease coverage by 0.05%.
The diff coverage is 100.00%.

@@             Coverage Diff             @@
##            master    #1686      +/-   ##
===========================================
- Coverage   100.00%   99.94%   -0.06%     
===========================================
  Files           21       21              
  Lines         3925     3948      +23     
  Branches       788      790       +2     
===========================================
+ Hits          3925     3946      +21     
- Misses           0        1       +1     
- Partials         0        1       +1

Impacted Files	Coverage Δ
pydantic/generics.py	`100.00% <100.00%> (ø)`
pydantic/typing.py	`98.11% <0.00%> (-1.89%)`	⬇️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 29e3877...028444b. Read the comment docs.

PrettyWood · 2020-08-07T12:19:42Z

@MrMrRobat I reckon your PR also fixes #1734 so #1757 can probably be closed. I let it open in the meantime but maybe you could add my test in your PR and add #1734 in closed issues ;)

tests/test_utils.py

Co-authored-by: PrettyWood <em.jolibois@gmail.com>

Bobronium · 2020-08-07T12:43:34Z

@PrettyWood, test I found in #1757 seems to work regardless of changes I made 🤔.
I assume example from #1734 will work as a test, as it reproduce the issue

PrettyWood · 2020-08-07T12:47:00Z

@MrMrRobat weird 🤔 I just tried with the test on master and it actually raises

>       globalns = sys.modules[cls.__module__].__dict__.copy()
E       KeyError: None

pydantic/main.py:684: KeyError

Bobronium · 2020-08-07T12:49:41Z

Ah, sorry, now I get it. I just removed setting model attribute to a module, but kept setting of a module to a model

… in create_model

Bobronium · 2020-08-25T18:49:13Z

So, pickling can't be done if model has no global reference somewhere in code. Pickle threats path in __qualname__ of as such reference.

Dynamically created models don't have global references therefore can not be pickled, unless referenced manually.

We can't set global references to each dynamically created model, as it will either override previous created model, or/and lead to memory leaks.

But turns out, it's not trivial task to determine if model was created globally or locally.

It can be done like this in normal conditions:

def is_call_from_module():
    import inspect

    return inspect.stack()[2].function == '<module>'

However, if code is executed in debugger, value of a inspect.stack()[2].function would be evaluate_expression. Obviously, we can't predict any situation like this.

So, we either need a stable way to detect such differences, or think about other implementations.

Bobronium · 2020-08-25T20:18:59Z

typing.Generic, for example, doesn't create new classes for concrete types, but instead uses special class instances and therefore can be pickled. However I'm not sure how it can be used for pydantic models.

Bobronium · 2020-08-29T07:33:09Z

I finally found the way to determine call from module, but it won't work with compiled code, but it mostly needed in generics, and since they won't work in compiled code anyway, it's not a problem.

However get_caller_module_name doesn't work in compiled code either, and that's bring inconsistency if we will use it in create_model

The question is: do we need that functionality for non-compiled code, or we better stick with the way it will work the same on both versions?

What will work in compiled and non-compiled code:

Add default value to create_model argument __module__=__name__ ('pydantic.main')
Add ensure_picklable function wicth will help to ensure that dynamically created models are pickable (all it actually does is create reference in model.__module__ by model.__name__, if it's not used by another object)

import pickle
from pydantic import create_model
# from pydantic.utils import ensure_picklable

# this should not be done this way. Reference should be set globally, not dynamically,
# otherwise there is no guarantee that reference will be created by the time model will be unpickled
# model = ensure_picklable(create_model('FooModel'))
# pickle.dumps(model)  # works

BarModel = create_model('BarModel', __module__=__name__)
pickle.dumps(BarModel)  # works

Additional things that will work in non-compiled code:

Determine in what module model was created, so this will work without specifying module:

BarModel = create_model('BarModel')
pickle.dumps(BarModel)

remidebette · 2020-09-12T06:33:49Z

Hi!

Is this blocked? Can we help?

Thank you

Bobronium · 2020-09-14T21:37:19Z

@remidebette, I'm trying to figure out consistent way of solving the problem acroas all platforms. Hacky staff with frames seems to not do the job. Hope, I'll have a success with implementing this with __reduce__ or __reduce_ex__ method, but need more time for that.

Until issue is resolved, you can still fix such issues by using create_model only on module level, declaring variable with the name of the model and using __module__ arg like this:

from pydantic import create_model

MyModel = create_model('MyModel', __module__=__name__)

Such models won't have any problems with pickling/forward refs/etc.

For generics you have to manually set reference to base model module, like so:

from typing import Generic, TypeVar

from pydantic.generics import GenericModel

T = TypeVar('T')
class MyGenericModel(Generic[T]. GenericModel):
    t: T

IntModel = MyGenericModel[int]
setattr(MyGenericModel.__module__, IntModel.__name__, IntModel)

Code above might be implemented in your base classe's __class_getitem__ like so:

class MyAutoGenericModel(Generic[T]. GenericModel):
    t: T

    def __class_getitem__(cls, *args, **kwargs):
        created_model = super().__class_getitem__(*args, **kwargs)
        setattr(cls.__module__, created_model.__name__, created_model)
        return created_model

remidebette · 2020-09-14T21:40:15Z

Thanks for the detailed explanation!

samuelcolvin

This is looking good, let me know if I can do anything to help fix it.

I assume it's not as simple as moving is_call_from_module to genetics.py which is never compiled?

samuelcolvin · 2020-10-08T18:54:48Z

pydantic/main.py

@@ -796,12 +796,12 @@ def __values__(self) -> 'DictStrAny':
 _is_base_model_class_defined = True


-def create_model(
+def create_model(  # noqa: C901 (ignore complexity)


is this needed if create_model hasn't been modified?

samuelcolvin · 2020-10-08T18:55:13Z

pydantic/utils.py

@@ -241,7 +243,8 @@ def unique_list(input_list: Union[List[T], Tuple[T, ...]]) -> List[T]:


 def update_normalized_all(
-    item: Union['AbstractSetIntStr', 'MappingIntStrAny'], all_items: Union['AbstractSetIntStr', 'MappingIntStrAny'],
+    item: Union['AbstractSetIntStr', 'MappingIntStrAny'],


anything changed?

seems like inconsistency between black versions. Which one should I use?

samuelcolvin · 2020-10-08T18:56:22Z

pydantic/utils.py

+    except IndexError:
+        raise RuntimeError('This function must be used inside another function')


Suggested change

except IndexError:

raise RuntimeError('This function must be used inside another function')

except IndexError as e:

raise RuntimeError('This function must be used inside another function') from e

samuelcolvin · 2020-10-08T18:56:31Z

pydantic/utils.py

+    except IndexError:
+        raise RuntimeError('This function must be used inside another function')


Suggested change

except IndexError:

raise RuntimeError('This function must be used inside another function')

except IndexError as e:

raise RuntimeError('This function must be used inside another function') from e

samuelcolvin · 2020-10-08T18:58:58Z

tests/test_utils.py

+
+
+@pytest.mark.skipif(
+    compiled,


is this skip required? Surely the code below in create_module is never compiled so should always work.

If is_call_from_module itself needs to be not compiled, I guess you need to move it into genetics.py?

Wow, somehow I never thought about this. I did so and CI just passed!

Just to be clear: compiled code may not contain any use of GenericModel, right?

samuelcolvin · 2020-10-08T19:00:28Z

tests/test_utils.py

+def get_current_module_name():
+    return get_caller_module_name()
+
+assert get_current_module_name() == __name__


Suggested change

assert get_current_module_name() == __name__

module_name = get_current_module_name()

assert module_name == __name__, (module_name, __name__)

so we can better understand the error?

doesn't pytest provide such introspection in case of error? 🤔

upd. it doesn't :(

I forgot to apply suggestions before moving functions and tests to generics, so have to make changes manually

samuelcolvin · 2020-10-09T09:31:54Z

this looks great, please could you:

add a change description
fix the coverage

Bobronium · 2020-10-09T10:47:32Z

@samuelcolvin, master CI is failing again 😕. I guess PR fromatted with different black version was merged and broke CI.

samuelcolvin · 2020-10-09T12:30:41Z

master should now be fixed.

…odule

Bobronium · 2020-10-10T18:01:09Z

@samuelcolvin, FYI: test_module fixture can be used to test try_assert.py without having a separate command for that:

def test_assert_in_validators(test_module):
    test_module("try_assert.py")

Upd. Actually, in case of try_assert.py, it could be simply renamed to test_assert.py and runned by pytest withoht any problems, just by putting """PYTEST_DONT_REWRITE""" on top of the module

As for current tests, I decided to rewrite them and enhanced create_module and added run_as_module that runs tests in subprocess

this will allow run modules in subprocess and define module code in functions-containers

…el with module definition in function

pydantic/main.py

* 'master' of https://github.com/samuelcolvin/pydantic: Add pickle support to dynamically created models and generics (pydantic#1686) feat(field): add `Enum` and `IntEnum` as valid types for fields (pydantic#1737) feat: add basic support for python 3.9 (pydantic#1844) Bump isort from 5.6.1 to 5.6.4 (pydantic#2013) feat(schema): support custom title, description and default for enums (pydantic#1749) chore(deps-dev): bump mypy from 0.782 to 0.790 (pydantic#1998)

PrettyWood mentioned this pull request Aug 7, 2020

feat: support update_forward_refs in scripts #1757

Closed

4 tasks

PrettyWood reviewed Aug 7, 2020

View reviewed changes

tests/test_utils.py Outdated Show resolved Hide resolved

Bobronium added 4 commits August 7, 2020 15:25

Add pickle support to dynamically created models

520df30

Add created model module check

4be648d

Use globals() to retrieve model

c4996ab

Use single quotes

3daa8df

Bobronium force-pushed the allow-dynamically-created-models-pickling branch from 8201b6e to 3daa8df Compare August 7, 2020 12:26

Bobronium and others added 2 commits August 7, 2020 15:28

use mocker instead of mock

4e12aa3

Co-authored-by: PrettyWood <em.jolibois@gmail.com>

remove unused import

bce2e64

Bobronium added 4 commits August 7, 2020 15:58

add test for dynamic forward ref

e415046

move generic test to test_generics.py

65b10f4

fix test_generic_model_pickle test, add additional check for <locals>…

1fab539

… in create_model

fix code style

61da678

resolve issues with global/local models

c84134a

make ensure_picklable return given model

a8b7599

Bobronium added 2 commits September 12, 2020 14:34

remove ensure_picklable, use test_is_call_from_module only in generics

666b763

reformat code after updating black==20.8b1

799fc88

Bobronium mentioned this pull request Sep 14, 2020

Dynamically create_model that contains forward refs #1734

Closed

samuelcolvin reviewed Oct 8, 2020

View reviewed changes

Bobronium and others added 2 commits October 9, 2020 02:26

move get_caller_module_name and is_call_from_module to generics

3905f39

Merge branch 'master' into allow-dynamically-created-models-pickling

620ce82

apply suggestions from @samuelcolvin

93d9714

Bobronium added 6 commits October 10, 2020 17:26

Merge branch 'master' into allow-dynamically-created-models-pickling

d80f92d

add tests for get_caller_module and is_call_from_module called from m…

46e6783

…odule

fix linting

db8271c

fix path to modules in test_module fixture, capture stderr and stdout

371e263

fix broken test module

85bc1ae

fix subprocess call for windows

08832c9

Bobronium added 7 commits October 11, 2020 14:47

enhance create_module fixture, add run_as_module fixture

473b069

this will allow run modules in subprocess and define module code in functions-containers

add test for redefined concrete model without cache

ebd1434

add changes file

f1424c4

remove unused variable

639c85d

rewrite tests with pytest.raises

727aa70

fix linting

1c9dfd7

rewrite test_create_model_pickle and test_forward_ref_with_create_mod…

5348748

…el with module definition in function

Bobronium commented Oct 11, 2020

View reviewed changes

pydantic/main.py Outdated Show resolved Hide resolved

Bobronium and others added 2 commits October 11, 2020 19:42

Update create_model docstring

2770773

Merge branch 'master' into allow-dynamically-created-models-pickling

028444b

samuelcolvin merged commit 6b53cab into pydantic:master Oct 18, 2020

bogdandm mentioned this pull request Oct 29, 2020

[perfomance regression] Generic model creation cause huge slowdown #2070

Closed

3 tasks

toichiserada mentioned this pull request Oct 20, 2021

Error during pickling (with Cloudpickle) of a instance of dynamic schema created with create_model() #3336

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add pickle support to dynamically created models and generics #1686

Add pickle support to dynamically created models and generics #1686

Bobronium commented Jul 5, 2020 •

edited

codecov bot commented Jul 5, 2020 •

edited

PrettyWood commented Aug 7, 2020 •

edited

Bobronium commented Aug 7, 2020

PrettyWood commented Aug 7, 2020

Bobronium commented Aug 7, 2020

Bobronium commented Aug 25, 2020 •

edited

Bobronium commented Aug 25, 2020

Bobronium commented Aug 29, 2020 •

edited

remidebette commented Sep 12, 2020

Bobronium commented Sep 14, 2020 •

edited

remidebette commented Sep 14, 2020

samuelcolvin left a comment

samuelcolvin Oct 8, 2020

samuelcolvin Oct 8, 2020

Bobronium Oct 8, 2020

samuelcolvin Oct 8, 2020

samuelcolvin Oct 8, 2020

samuelcolvin Oct 8, 2020

Bobronium Oct 8, 2020

samuelcolvin Oct 8, 2020

Bobronium Oct 8, 2020 •

edited

Bobronium Oct 8, 2020

samuelcolvin commented Oct 9, 2020

Bobronium commented Oct 9, 2020

samuelcolvin commented Oct 9, 2020

Bobronium commented Oct 10, 2020 •

edited

		except IndexError:
		raise RuntimeError('This function must be used inside another function')

	assert get_current_module_name() == __name__
	module_name = get_current_module_name()
	assert module_name == __name__, (module_name, __name__)



		@pytest.mark.skipif(
		compiled,

Add pickle support to dynamically created models and generics #1686

Add pickle support to dynamically created models and generics #1686

Conversation

Bobronium commented Jul 5, 2020 • edited

Change Summary

Related issue number

Checklist

codecov bot commented Jul 5, 2020 • edited

Codecov Report

PrettyWood commented Aug 7, 2020 • edited

Bobronium commented Aug 7, 2020

PrettyWood commented Aug 7, 2020

Bobronium commented Aug 7, 2020

Bobronium commented Aug 25, 2020 • edited

Bobronium commented Aug 25, 2020

Bobronium commented Aug 29, 2020 • edited

remidebette commented Sep 12, 2020

Bobronium commented Sep 14, 2020 • edited

remidebette commented Sep 14, 2020

samuelcolvin left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Bobronium Oct 8, 2020 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

samuelcolvin commented Oct 9, 2020

Bobronium commented Oct 9, 2020

samuelcolvin commented Oct 9, 2020

Bobronium commented Oct 10, 2020 • edited

Bobronium commented Jul 5, 2020 •

edited

codecov bot commented Jul 5, 2020 •

edited

PrettyWood commented Aug 7, 2020 •

edited

Bobronium commented Aug 25, 2020 •

edited

Bobronium commented Aug 29, 2020 •

edited

Bobronium commented Sep 14, 2020 •

edited

Bobronium Oct 8, 2020 •

edited

Bobronium commented Oct 10, 2020 •

edited