Skip to content
This repository has been archived by the owner on Jul 2, 2021. It is now read-only.

Fix PickableSequentialChain.copy() #764

Merged
merged 3 commits into from
Feb 6, 2019

Conversation

ktns
Copy link
Contributor

@ktns ktns commented Feb 2, 2019

On master, it seems that modification on copied object by PickableSequentialChain.copy() affects original object, due to shallow-copied layer_names and _pick.

This PR adds tests about it and fixes it.

@yuyu2172
Copy link
Member

yuyu2172 commented Feb 4, 2019

Could you copy _return_tuple too?

@ktns
Copy link
Contributor Author

ktns commented Feb 5, 2019

Because it seems that _return_tuple is supposed to be a bool value, isn't it better to explicitly sanitize it like copied._return_tuple = bool(self._return_tuple) ?

@ktns
Copy link
Contributor Author

ktns commented Feb 5, 2019

Sorry, I've changed my mind. Now I think it's better to copy _return_tuple also in case the usage of _return_tuple changes in future.

@knorth55 knorth55 added the bug label Feb 5, 2019
@knorth55 knorth55 added this to the 0.12 milestone Feb 5, 2019
@yuyu2172
Copy link
Member

yuyu2172 commented Feb 6, 2019

Thanks for the bug fix!

@yuyu2172 yuyu2172 merged commit 9d23308 into chainer:master Feb 6, 2019
@ktns ktns deleted the fix_pickable_sequential_chain_copy branch February 8, 2019 07:05
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants