Skip to content

Conversation

@copybara-service
Copy link

@copybara-service copybara-service bot commented Dec 17, 2020

Make Gin dynamic class wrapping compatible with pickle.

A new take on how to do this, since the wrapt approach also didn't end up working. This approach does the following:

  • When wrapping a class where mutation should be disallowed, this dynamically subclasses the class's metaclass, and intercepts/wraps its __call__.
  • However, this metaclass then returns an instance of the original type, which allows pickling to work.

To preserve proper signature checking, there are a few changes also to the general Gin wrapper function, to always use the signature associated with the right __init__/__new__ function.

As a result of this change, it's no longer permissible to subclass the wrapped class returned by gin.external_configurable, so the test case for this has been removed. This should be an ok tradeoff, as no one is relying on this behavior and it's a pretty dubious use case to begin with.

@google-cla google-cla bot added the cla: yes label Dec 17, 2020
@copybara-service copybara-service bot force-pushed the test_348047963 branch 4 times, most recently from ca5caf8 to 1010954 Compare December 17, 2020 22:00
A new take on how to do this, since the wrapt approach also didn't end up working. This approach does the following:

 - When wrapping a class where mutation should be disallowed, this dynamically subclasses the class's metaclass, and intercepts/wraps its `__call__`.
 - However, this metaclass then returns an instance of the *original* type, which allows pickling to work.

To preserve proper signature checking, there are a few changes also to the general Gin wrapper function, to always use the signature associated with the right `__init__`/`__new__` function.

As a result of this change, it's no longer permissible to subclass the wrapped class returned by gin.external_configurable, so the test case for this has been removed. This should be an ok tradeoff, as no one is relying on this behavior and it's a pretty dubious use case to begin with.

PiperOrigin-RevId: 348525899
@copybara-service copybara-service bot merged commit bf622d7 into master Dec 21, 2020
@copybara-service copybara-service bot deleted the test_348047963 branch December 21, 2020 22:13
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant