Skip to content
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

[BE] wrap deprecated function/class with typing_extensions.deprecated #126898

Closed
wants to merge 8 commits into from

Conversation

XuehaiPan
Copy link
Collaborator

@XuehaiPan XuehaiPan commented May 22, 2024

Use typing_extensions.deprecated for deprecation annotation if possible. Otherwise, add category=FutureWarning to warnings.warn("message") if the category is missing.

Note that only warnings that their messages contain [Dd]eprecat(ed|ion) are updated in this PR.

UPDATE: Use FutureWarning instead of DeprecationWarning.

Resolves #126888

cc @mrshenli @pritamdamania87 @zhaojuanmao @satgera @gqchen @aazzolini @osalpekar @jiayisuse @H-Huang @kwen2501 @awgu @penguinwu @fegin @XilunWu @wanchaol @fduwjj @wz337 @tianyu-l @wconstab @yf225 @chauhang @d4l3k @jgong5 @mingfeima @XiaobingSuper @sanchitintel @ashokei @jingxu10 @ezyang @malfet @xuzhao9 @gramster @mcarilli @ptrblck @leslie-fang-intel @voznesenskym @EikanWang @Guobing-Chen @zhuhaozhe @blzheng @wenzhe-nrv @jiayisunx @peterbell10 @ipiszy @chenyang78 @kadeng @muchulee8 @ColinPeppler @amjames @desertfire @LucasLLC

@XuehaiPan XuehaiPan added module: typing Related to mypy type annotations better-engineering Relatively self-contained tasks for better engineering contributors module: deprecation topic: deprecation topic category labels May 22, 2024
Copy link

pytorch-bot bot commented May 22, 2024

🔗 Helpful Links

🧪 See artifacts and rendered test results at hud.pytorch.org/pr/126898

Note: Links to docs will display an error until the docs builds have been completed.

❌ 109 New Failures, 32 Unrelated Failures

As of commit 2653d0c with merge base 114c752 (image):

NEW FAILURES - The following jobs have failed:

FLAKY - The following jobs failed but were likely due to flakiness present on trunk:

UNSTABLE - The following jobs failed but were likely due to flakiness present on trunk and has been marked as unstable:

This comment was automatically generated by Dr. CI and updates every 15 minutes.

@pytorch-bot pytorch-bot bot added ciflow/inductor module: cpu CPU specific problem (e.g., perf, algorithm) module: distributed_checkpoint module: dynamo oncall: distributed Add this issue/PR to distributed oncall triage queue release notes: quantization release notes category release notes: AO frontend labels May 22, 2024
@mikaylagawarecki mikaylagawarecki removed their request for review May 22, 2024 20:01
Copy link
Collaborator

@albanD albanD left a comment

Choose a reason for hiding this comment

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

I don't think we should use DeprecationWarning in general.
These warnings types are hidden by default and we explicitly want users to see these warnings so they update their code.

So we would need to update the calls using deprecated() to use UserWarning and continue to use that for all other warnings.

torch/_dynamo/external_utils.py Outdated Show resolved Hide resolved
@bdhirsh bdhirsh added the triaged This issue has been looked at a team member, and triaged and prioritized into an appropriate module label May 23, 2024
@XuehaiPan
Copy link
Collaborator Author

I don't think we should use DeprecationWarning in general. These warnings types are hidden by default and we explicitly want users to see these warnings so they update their code.

After referring to the Python documentation, I noticed that DeprecationWarnings is ignored by default (except DeprecationWarnings warned in __main__).

exception DeprecationWarning
Base class for warnings about deprecated features when those warnings are intended for other Python developers.

Ignored by the default warning filters, except in the main module (PEP 565). Enabling the Python Development Mode shows this warning.

I update DeprecationWarnings to FutureWarnings in this PR.

Warning Categories

Changed in version 3.7: Previously DeprecationWarning and FutureWarning were distinguished based on whether a feature was being removed entirely or changing its behaviour. They are now distinguished based on their intended audience and the way they’re handled by the default warnings filters.


So we would need to update the calls using deprecated() to use UserWarning and continue to use that for all other warnings.

Based on the documentation, I'd prefer FutureWarning instead of UserWarning. Note that only warnings that their messages contain [Dd]eprecat(ed|ion) are updated in this PR.

exception UserWarning
Base class for warnings generated by user code.

exception FutureWarning
Base class for warnings about deprecated features when those warnings are intended for end users of applications that are written in Python.

Aidyn-A pushed a commit to tinglvv/pytorch that referenced this pull request May 30, 2024
…d` (pytorch#126898)

Use `typing_extensions.deprecated` for deprecation annotation if possible. Otherwise, add `category=FutureWarning` to `warnings.warn("message")` if the category is missing.

Note that only warnings that their messages contain `[Dd]eprecat(ed|ion)` are updated in this PR.

UPDATE: Use `FutureWarning` instead of `DeprecationWarning`.

Resolves pytorch#126888

- pytorch#126888

Pull Request resolved: pytorch#126898
Approved by: https://github.com/albanD
pytorchmergebot added a commit that referenced this pull request May 31, 2024
…127433)"

This reverts commit 6e0eeec.

Reverted #127433 on behalf of https://github.com/fbgheith due to depends on #126898 which is failing internally and needs to be reverted ([comment](#127433 (comment)))
@fbgheith
Copy link
Contributor

@pytorchbot revert -m "switching typing-extensions=4.3.0 to 4.9.0 causes internal failure" -c ghfirst

Here is the failures:

 1) caffe2.torch.fb.module_factory.sync_sgd.tests.test_lightning_module.SyncSGDLightningModuleTest: test_load_state_dict
    1) RuntimeError: 
    undefined value torch:
      File "/data/sandcastle/boxes/eden-trunk-hg-full-fbsource/buck-out/v2/gen/fbcode/05952b5e2c28a7bd/caffe2/torch/fb/module_factory/sync_sgd/tests/__test_lightning_module__/test_lightning_module#link-tree/typing_extensions.py", line 32
        It will depend on the context where to use what.
        """
        return torch.compiler.is_compiling()
               ~~~~~ <--- HERE
    'is_compiling' is being compiled since it was called from 'SplitTableBatchedEmbeddingBagsCodegen._generate_vbe_metadata'
      File "/data/sandcastle/boxes/eden-trunk-hg-full-fbsource/buck-out/v2/gen/fbcode/05952b5e2c28a7bd/caffe2/torch/fb/module_factory/sync_sgd/tests/__test_lightning_module__/test_lightning_module#link-tree/fbgemm_gpu/split_table_batched_embeddings_ops_training.py", line 1132
        
                max_B = total_batch_size_per_feature.max().item()

Will require some internal shepherding.

@pytorchmergebot
Copy link
Collaborator

@pytorchbot successfully started a revert job. Check the current status here.
Questions? Feedback? Please reach out to the PyTorch DevX Team

pytorchmergebot added a commit that referenced this pull request May 31, 2024
…eprecated` (#126898)"

This reverts commit 749a132.

Reverted #126898 on behalf of https://github.com/fbgheith due to switching typing-extensions=4.3.0 to 4.9.0 causes internal failure ([comment](#126898 (comment)))
@pytorchmergebot
Copy link
Collaborator

@XuehaiPan your PR has been successfully reverted.

@XuehaiPan
Copy link
Collaborator Author

@XuehaiPan XuehaiPan closed this Jun 1, 2024
pytorchmergebot pushed a commit that referenced this pull request Jun 2, 2024
…d` (#127689)

Use `typing_extensions.deprecated` for deprecation annotation if possible. Otherwise, add `category=FutureWarning` to `warnings.warn("message")` if the category is missing.

Note that only warnings that their messages contain `[Dd]eprecat(ed|ion)` are updated in this PR.

Resolves #126888

- #126888

This PR is split from PR #126898.

- #126898

------

Pull Request resolved: #127689
Approved by: https://github.com/Skylion007
bigfootjon pushed a commit that referenced this pull request Jun 5, 2024
…d` (#126898)

Use `typing_extensions.deprecated` for deprecation annotation if possible. Otherwise, add `category=FutureWarning` to `warnings.warn("message")` if the category is missing.

Note that only warnings that their messages contain `[Dd]eprecat(ed|ion)` are updated in this PR.

UPDATE: Use `FutureWarning` instead of `DeprecationWarning`.

Resolves #126888

- #126888

Pull Request resolved: #126898
Approved by: https://github.com/albanD

(cherry picked from commit 749a132)
bigfootjon pushed a commit that referenced this pull request Jun 5, 2024
…eprecated` (#126898)"

This reverts commit 749a132.

Reverted #126898 on behalf of https://github.com/fbgheith due to switching typing-extensions=4.3.0 to 4.9.0 causes internal failure ([comment](#126898 (comment)))

(cherry picked from commit 033e733)
bigfootjon pushed a commit that referenced this pull request Jun 5, 2024
…d` (#127689)

Use `typing_extensions.deprecated` for deprecation annotation if possible. Otherwise, add `category=FutureWarning` to `warnings.warn("message")` if the category is missing.

Note that only warnings that their messages contain `[Dd]eprecat(ed|ion)` are updated in this PR.

Resolves #126888

- #126888

This PR is split from PR #126898.

- #126898

------

Pull Request resolved: #127689
Approved by: https://github.com/Skylion007

(cherry picked from commit 67ef268)
petrex pushed a commit to petrex/pytorch that referenced this pull request Jun 5, 2024
…ytorch#127433)"

This reverts commit 6e0eeec.

Reverted pytorch#127433 on behalf of https://github.com/fbgheith due to depends on pytorch#126898 which is failing internally and needs to be reverted ([comment](pytorch#127433 (comment)))
petrex pushed a commit to petrex/pytorch that referenced this pull request Jun 5, 2024
…eprecated` (pytorch#126898)"

This reverts commit 749a132.

Reverted pytorch#126898 on behalf of https://github.com/fbgheith due to switching typing-extensions=4.3.0 to 4.9.0 causes internal failure ([comment](pytorch#126898 (comment)))
petrex pushed a commit to petrex/pytorch that referenced this pull request Jun 5, 2024
…d` (pytorch#127689)

Use `typing_extensions.deprecated` for deprecation annotation if possible. Otherwise, add `category=FutureWarning` to `warnings.warn("message")` if the category is missing.

Note that only warnings that their messages contain `[Dd]eprecat(ed|ion)` are updated in this PR.

Resolves pytorch#126888

- pytorch#126888

This PR is split from PR pytorch#126898.

- pytorch#126898

------

Pull Request resolved: pytorch#127689
Approved by: https://github.com/Skylion007
pytorchmergebot pushed a commit that referenced this pull request Jun 8, 2024
…tils.is_compiling()` (#127690)

This PR is split from PR #126898.

- #126898

------

Pull Request resolved: #127690
Approved by: https://github.com/Skylion007
pytorchmergebot added a commit that referenced this pull request Jun 10, 2024
…ternal_utils.is_compiling()` (#127690)"

This reverts commit 348b181.

Reverted #127690 on behalf of https://github.com/clee2000 due to sorry I think #126898 (comment) is still relevant, I will reach out to them to see what needs to be done in internal to get this remerged ([comment](#127690 (comment)))
TharinduRusira pushed a commit to TharinduRusira/pytorch that referenced this pull request Jun 14, 2024
TharinduRusira pushed a commit to TharinduRusira/pytorch that referenced this pull request Jun 14, 2024
…ternal_utils.is_compiling()` (pytorch#127690)"

This reverts commit 348b181.

Reverted pytorch#127690 on behalf of https://github.com/clee2000 due to sorry I think pytorch#126898 (comment) is still relevant, I will reach out to them to see what needs to be done in internal to get this remerged ([comment](pytorch#127690 (comment)))
@XuehaiPan XuehaiPan deleted the warnings-deprecated branch June 14, 2024 19:29
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
better-engineering Relatively self-contained tasks for better engineering contributors ciflow/inductor ciflow/trunk Trigger trunk jobs on your pull request keep-going Don't stop on first failure, keep running tests until the end Merged module: amp (automated mixed precision) autocast module: cpu CPU specific problem (e.g., perf, algorithm) module: deprecation module: distributed_checkpoint module: dynamo module: inductor module: typing Related to mypy type annotations oncall: distributed Add this issue/PR to distributed oncall triage queue open source release notes: AO frontend release notes: quantization release notes category Reverted topic: deprecation topic category triaged This issue has been looked at a team member, and triaged and prioritized into an appropriate module
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[BE] wrap deprecated function/class with typing_extensions.deprecated for better IDE integration
9 participants