Skip to content

Raise warning for unpickable local function #547

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
wants to merge 1 commit into from

Conversation

ejguan
Copy link
Contributor

@ejguan ejguan commented Jun 24, 2022

Summary:
Fixes #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.
>>> 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

@facebook-github-bot facebook-github-bot added CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed. fb-exported labels Jun 24, 2022
@facebook-github-bot
Copy link
Contributor

This pull request was exported from Phabricator. Differential Revision: D37417556

1 similar comment
@facebook-github-bot
Copy link
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: pytorch#547

Fixes 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 ejguan force-pushed the export-D37417556 branch from 0eebef7 to 0d46374 Compare June 24, 2022 17:47
ejguan added a commit to ejguan/data that referenced this pull request Jun 24, 2022
Summary:
X-link: pytorch/pytorch#80232

Pull Request resolved: pytorch#547

Fixes 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
@ejguan ejguan force-pushed the export-D37417556 branch from 0d46374 to 0cc4106 Compare June 24, 2022 18:55
@facebook-github-bot
Copy link
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: pytorch#547

Fixes 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
@ejguan ejguan force-pushed the export-D37417556 branch from 0cc4106 to 16f963e Compare June 24, 2022 18:59
@facebook-github-bot
Copy link
Contributor

This pull request was exported from Phabricator. Differential Revision: D37417556

Summary:
X-link: pytorch/pytorch#80232

Pull Request resolved: pytorch#547

Fixes 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: pytorch/data#547

Fixes 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
@ejguan ejguan force-pushed the export-D37417556 branch from 16f963e to ad3474e Compare June 27, 2022 16:19
@facebook-github-bot
Copy link
Contributor

This pull request was exported from Phabricator. Differential Revision: D37417556

@NivekT NivekT self-requested a review June 27, 2022 17:49
Copy link
Contributor

@NivekT NivekT left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The CI failure is due to pytorch/pytorch#80232 isn't in nightly?

@ejguan
Copy link
Contributor Author

ejguan commented Jun 27, 2022

The CI failure is due to pytorch/pytorch#80232 isn't in nightly?

@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: pytorch/data#547

Fixes 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: pytorch/data#547

Fixes 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
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed. fb-exported
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Warn about pickle-ablity when using dp.map(some_local_function) ?
3 participants