make torchdata default imports lazy #1243
Conversation
facebook-github-bot
added
the
CLA Signed
|
Not sure why CI is skipped |
|
@andrewkho has imported this pull request. If you are a Meta employee, you can view this diff on Phabricator. |
|
CI failing for MacOS during test-documentation phase for S3 connector with some CPP Stacktrace: |
|
The S3FileSaver seems to pass on MacOS when reverting the change, I'll try it locally on my laptop to see if I can repro |
|
I see, like needed to fix this import _extension which will make the s3 binary available |
|
@andrewkho has imported this pull request. If you are a Meta employee, you can view this diff on Phabricator. |
|
@andrewkho has imported this pull request. If you are a Meta employee, you can view this diff on Phabricator. |
|
Test is currently failing on macos for the multiprocess iterdatapipe test, when ctx == "fork". A similar test with non-datapipes is also skipped for macOS so going to follow that here too |
| @@ -1638,6 +1638,7 @@ def test_multiprocessing_contexts(self): | |||
| list(self._get_data_loader(ds_cls(counting_ds_n), multiprocessing_context=ctx, **dl_common_args)), | |||
| ) | |||
|
|
|||
| @unittest.skipIf(IS_MACOS, "Not working on macos") | |||
There was a problem hiding this comment.
See test_multiprocessing_contexts() above on line 1614, similar non-datapipe test is skipped for macOS so going to follow that here too
There was a problem hiding this comment.
An alternative for this test to pass is to skip ctx == "fork" or locally import torchdata.datapipes, even though there is absolutely no dependency on it, guessing it's something to do with https://www.wefearchange.org/2018/11/forkmacos.rst.html
|
@andrewkho has imported this pull request. If you are a Meta employee, you can view this diff on Phabricator. |
|
@andrewkho merged this pull request in db1eae3. |
Summary: Push all imports in torchdata/init.py to be lazy by default so they don't interfere with new development (eg stateful_dataloader)
Test Plan:
New unit test, CI for the rest
Please read through our contribution guide prior to
creating your pull request.
Fixes #{issue number}
Changes
Makes imports lazy, should be no-op for downstream users