Raise warning for unpickable local function#547
Closed
ejguan wants to merge 1 commit intometa-pytorch:mainfrom
Closed
Raise warning for unpickable local function#547ejguan wants to merge 1 commit intometa-pytorch:mainfrom
ejguan wants to merge 1 commit intometa-pytorch:mainfrom
Conversation
Contributor
|
This pull request was exported from Phabricator. Differential Revision: D37417556 |
1 similar comment
Contributor
|
This pull request was exported from Phabricator. Differential Revision: D37417556 |
ejguan
added a commit
to ejguan/data
that referenced
this pull request
Jun 24, 2022
Summary: Pull Request resolved: meta-pytorch#547 Fixes meta-pytorch#538 - Improve the validation function to raise warning about unpickable function when either lambda or local function is provided to DataPipe. - The inner function from functools.partial object is extracted as well for validation - Mimic the behavior of pickle module for local lambda function: It would only raise Error for the local function rather than lambda function. So, we will raise warning about local function not lambda function. ```py >>> import pickle >>> def fn(): ... lf = lambda x: x ... pickle.dumps(lf) >>> pickle.dumps(fn) AttributeError: Can't pickle local object 'fn.<locals>.<lambda>' ``` This Diff also fixes the Error introduced by pytorch/pytorch#79344 Differential Revision: D37417556 fbshipit-source-id: 4877a7de9a2060f6da499dc26b7de64b4a200cd7
ejguan
added a commit
to ejguan/data
that referenced
this pull request
Jun 24, 2022
Summary: X-link: pytorch/pytorch#80232 Pull Request resolved: meta-pytorch#547 Fixes meta-pytorch#538 - Improve the validation function to raise warning about unpickable function when either lambda or local function is provided to DataPipe. - The inner function from functools.partial object is extracted as well for validation - Mimic the behavior of pickle module for local lambda function: It would only raise Error for the local function rather than lambda function. So, we will raise warning about local function not lambda function. ```py >>> import pickle >>> def fn(): ... lf = lambda x: x ... pickle.dumps(lf) >>> pickle.dumps(fn) AttributeError: Can't pickle local object 'fn.<locals>.<lambda>' ``` This Diff also fixes the Error introduced by pytorch/pytorch#79344 Differential Revision: D37417556 fbshipit-source-id: 7213ee84b34092e0c2cf293ff8bf1dc56659fc83
Contributor
|
This pull request was exported from Phabricator. Differential Revision: D37417556 |
ejguan
added a commit
to ejguan/data
that referenced
this pull request
Jun 24, 2022
Summary: X-link: pytorch/pytorch#80232 Pull Request resolved: meta-pytorch#547 Fixes meta-pytorch#538 - Improve the validation function to raise warning about unpickable function when either lambda or local function is provided to DataPipe. - The inner function from functools.partial object is extracted as well for validation - Mimic the behavior of pickle module for local lambda function: It would only raise Error for the local function rather than lambda function. So, we will raise warning about local function not lambda function. ```py >>> import pickle >>> def fn(): ... lf = lambda x: x ... pickle.dumps(lf) >>> pickle.dumps(fn) AttributeError: Can't pickle local object 'fn.<locals>.<lambda>' ``` This Diff also fixes the Error introduced by pytorch/pytorch#79344 Differential Revision: D37417556 fbshipit-source-id: c17e475bde703f2f55af140f2155d41efb45f049
Contributor
|
This pull request was exported from Phabricator. Differential Revision: D37417556 |
Summary: X-link: pytorch/pytorch#80232 Pull Request resolved: meta-pytorch#547 Fixes meta-pytorch#538 - Improve the validation function to raise warning about unpickable function when either lambda or local function is provided to DataPipe. - The inner function from functools.partial object is extracted as well for validation - Mimic the behavior of pickle module for local lambda function: It would only raise Error for the local function rather than lambda function. So, we will raise warning about local function not lambda function. ```py >>> import pickle >>> def fn(): ... lf = lambda x: x ... pickle.dumps(lf) >>> pickle.dumps(fn) AttributeError: Can't pickle local object 'fn.<locals>.<lambda>' ``` This Diff also fixes the Error introduced by pytorch/pytorch#79344 Reviewed By: NivekT Differential Revision: D37417556 fbshipit-source-id: 6babb73a7daad470ec93c3bd0a0c08d71849d3c8
ejguan
added a commit
to ejguan/pytorch
that referenced
this pull request
Jun 27, 2022
Summary: Pull Request resolved: pytorch#80232 X-link: meta-pytorch/data#547 Fixes meta-pytorch/data#538 - Improve the validation function to raise warning about unpickable function when either lambda or local function is provided to DataPipe. - The inner function from functools.partial object is extracted as well for validation - Mimic the behavior of pickle module for local lambda function: It would only raise Error for the local function rather than lambda function. So, we will raise warning about local function not lambda function. ```py >>> import pickle >>> def fn(): ... lf = lambda x: x ... pickle.dumps(lf) >>> pickle.dumps(fn) AttributeError: Can't pickle local object 'fn.<locals>.<lambda>' ``` This Diff also fixes the Error introduced by pytorch#79344 Test Plan: ``` buck test //caffe2/test:datapipe buck test //pytorch/data/test:tests ``` Tested in OSS ``` # PT pytest test/test_datapipe.py -v # TD pytest test/test_iterdatapipe.py -v pytest test/test_mapdatapipe.py -v pytest test/test_serialization.py -v # TV pytest test/test_prototype_builtin_datasets.py -v ``` Reviewed By: NivekT Differential Revision: D37417556 fbshipit-source-id: 6fae4059285b8c742feda739cc5fe590b2e20c5e
Contributor
|
This pull request was exported from Phabricator. Differential Revision: D37417556 |
NivekT
approved these changes
Jun 27, 2022
Contributor
NivekT
left a comment
There was a problem hiding this comment.
The CI failure is due to pytorch/pytorch#80232 isn't in nightly?
Contributor
Author
@NivekT You are right. As long as the tests in phabricator pass, we should be fine. |
pytorchmergebot
pushed a commit
to pytorch/pytorch
that referenced
this pull request
Jun 27, 2022
Summary: X-link: meta-pytorch/data#547 Fixes meta-pytorch/data#538 - Improve the validation function to raise warning about unpickable function when either lambda or local function is provided to DataPipe. - The inner function from functools.partial object is extracted as well for validation - Mimic the behavior of pickle module for local lambda function: It would only raise Error for the local function rather than lambda function. So, we will raise warning about local function not lambda function. ```py >>> import pickle >>> def fn(): ... lf = lambda x: x ... pickle.dumps(lf) >>> pickle.dumps(fn) AttributeError: Can't pickle local object 'fn.<locals>.<lambda>' ``` This Diff also fixes the Error introduced by #79344 Test Plan: CI on PyTorch and TorchData Manually validated the tests from TorchVision Differential Revision: D37417556 Pull Request resolved: #80232 Approved by: https://github.com/NivekT
facebook-github-bot
pushed a commit
to pytorch/pytorch
that referenced
this pull request
Jun 27, 2022
Summary: Pull Request resolved: #80232 X-link: meta-pytorch/data#547 Fixes meta-pytorch/data#538 - Improve the validation function to raise warning about unpickable function when either lambda or local function is provided to DataPipe. - The inner function from functools.partial object is extracted as well for validation - Mimic the behavior of pickle module for local lambda function: It would only raise Error for the local function rather than lambda function. So, we will raise warning about local function not lambda function. ```py >>> import pickle >>> def fn(): ... lf = lambda x: x ... pickle.dumps(lf) >>> pickle.dumps(fn) AttributeError: Can't pickle local object 'fn.<locals>.<lambda>' ``` This Diff also fixes the Error introduced by #79344 Test Plan: ``` buck test //caffe2/test:datapipe buck test //pytorch/data/test:tests ``` Tested in OSS ``` # PT pytest test/test_datapipe.py -v # TD pytest test/test_iterdatapipe.py -v pytest test/test_mapdatapipe.py -v pytest test/test_serialization.py -v # TV pytest test/test_prototype_builtin_datasets.py -v ``` Reviewed By: NivekT Differential Revision: D37417556 fbshipit-source-id: 388d04004da43aaf14729cbe465a9247bef6c780
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary:
Fixes #538
This Diff also fixes the Error introduced by pytorch/pytorch#79344
Differential Revision: D37417556