Skip to content

Defaulting streaming dataset version to 1 and add a deprecation warning #1532

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 3 commits into from
Sep 18, 2022
Merged

Defaulting streaming dataset version to 1 and add a deprecation warning #1532

merged 3 commits into from
Sep 18, 2022

Conversation

karan6181
Copy link
Contributor

Description

  • Defaulting streaming dataset version to 1
  • Added a deprecation warning if using streaming dataset version 1
  • Make DeprecationWarning stack_level to 2 since the caller calls a function wrapper.

@karan6181 karan6181 requested a review from knighton as a code owner September 16, 2022 17:54
karan6181
karan6181 commented Sep 16, 2022
View reviewed changes
karan6181
karan6181 commented Sep 16, 2022
View reviewed changes
@karan6181
Copy link
Contributor Author

karan6181 commented Sep 16, 2022 •
edited
Loading

CI is failing since some of the test is failing with AttributeError: module 'distutils' has no attribute 'version'. This PR doesn't added a new package or change any existing one. I ran the test locally after doing pip install -e .[all] and they are passing. Below is the snapshot of some of the tests

tests/callbacks/test_loggers_across_callbacks.py::test_loggers_on_callbacks[CheckpointSaver-TensorboardLogger] PASSED
tests/callbacks/test_loggers_across_callbacks.py::test_loggers_on_callbacks[EarlyStopper-TensorboardLogger] PASSED
tests/callbacks/test_loggers_across_callbacks.py::test_loggers_on_callbacks[ExportForInferenceCallback-TensorboardLogger] PASSED
tests/callbacks/test_callbacks.py::TestCallbacks::test_multiple_init_and_close[TensorboardLogger] PASSED

PR #1534 is also failing with the same issue. Looks like something is wrong.

bandish-shah
bandish-shah approved these changes Sep 17, 2022
View reviewed changes
Copy link
Contributor

@bandish-shah bandish-shah left a comment

Choose a reason for hiding this comment

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

LGTM

@bandish-shah bandish-shah merged commit 3f3ba30 into mosaicml:dev Sep 18, 2022
@karan6181
Copy link
Contributor Author

CI is failing since some of the test is failing with AttributeError: module 'distutils' has no attribute 'version'. This PR doesn't added a new package or change any existing one. I ran the test locally after doing pip install -e .[all] and they are passing. Below is the snapshot of some of the tests

tests/callbacks/test_loggers_across_callbacks.py::test_loggers_on_callbacks[CheckpointSaver-TensorboardLogger] PASSED
tests/callbacks/test_loggers_across_callbacks.py::test_loggers_on_callbacks[EarlyStopper-TensorboardLogger] PASSED
tests/callbacks/test_loggers_across_callbacks.py::test_loggers_on_callbacks[ExportForInferenceCallback-TensorboardLogger] PASSED
tests/callbacks/test_callbacks.py::TestCallbacks::test_multiple_init_and_close[TensorboardLogger] PASSED

PR #1534 is also failing with the same issue. Looks like something is wrong.

Fixed by PR: #1536

@karan6181 karan6181 deleted the dataset_deprecation branch September 18, 2022 01:09
bandish-shah pushed a commit that referenced this pull request Sep 19, 2022
@karan6181
Defaulting streaming dataset version to 1 and add a deprecation warni…
002e799 
…ng (#1532)
bandish-shah pushed a commit to bandish-shah/composer that referenced this pull request Sep 19, 2022
@karan6181
Defaulting streaming dataset version to 1 and add a deprecation warni…
4ead520 
…ng (mosaicml#1532)
bcui19 pushed a commit to bcui19/composer that referenced this pull request Sep 22, 2022
@karan6181 @bcui19
Defaulting streaming dataset version to 1 and add a deprecation warni…
e19a56a 
…ng (mosaicml#1532)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@hanlint hanlint hanlint approved these changes

@bandish-shah bandish-shah bandish-shah approved these changes

@knighton knighton Awaiting requested review from knighton

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@karan6181 @hanlint @bandish-shah