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

Temporary fix to be BC compatible with torchtext #6168

Closed
wants to merge 1 commit into from

Conversation

thiagocrepaldi
Copy link

What does this PR do?

torchtext.data.Batch was deprecated (and moved to torchtext.legacy.data.Batch, which breaks PyTorch Lightning

Until PyTorch Lightning can use the new API, it should use the legacy version to allow PyTorch nightly (torch, torchtext) users to keep using PyTorch lightning during development

Fixes #6165

@codecov
Copy link

codecov bot commented Feb 24, 2021

Codecov Report

Merging #6168 (d6c2d23) into master (45158aa) will decrease coverage by 45%.
The diff coverage is 0%.

@@           Coverage Diff            @@
##           master   #6168     +/-   ##
========================================
- Coverage      91%     46%    -45%     
========================================
  Files         160     160             
  Lines       11435   11323    -112     
========================================
- Hits        10446    5260   -5186     
- Misses        989    6063   +5074     

Copy link
Contributor

@carmocca carmocca left a comment

Choose a reason for hiding this comment

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

Until PyTorch Lightning can use the new API

What's blocking it?

Can you look at the failing tests?

@thiagocrepaldi
Copy link
Author

thiagocrepaldi commented Feb 25, 2021

Until PyTorch Lightning can use the new API

What's blocking it?

Can you look at the failing tests?

It seems the CI VMs are using older torchtext:

torch                     1.4.0
torchtext                 0.6.0
torchvision               0.5.0

@Borda
Copy link
Member

Borda commented Feb 25, 2021

It seems the CI VMs are using older torchtext

it is expected, see min requirements
https://github.com/PyTorchLightning/pytorch-lightning/blob/ddf55a2f6a61c28c7d86c1144652c5e7c3811a3e/requirements/extra.txt#L6

@Borda
Copy link
Member

Borda commented Feb 26, 2021

seems to be duplicate to #6211

@Borda Borda added bug Something isn't working duplicate This issue or pull request already exists labels Feb 26, 2021
@Borda Borda requested review from tchaton and carmocca February 26, 2021 12:42
@Borda Borda closed this Feb 26, 2021
tiborkiss added a commit to tiborkiss/NeMo that referenced this pull request Mar 9, 2021
tiborkiss added a commit to tiborkiss/NeMo that referenced this pull request Mar 9, 2021
… pull down 1.8.0. Lightning-AI/pytorch-lightning#6168

Signed-off-by: Tibor Kiss <kiss.tibor@gmail.com>
tiborkiss added a commit to tiborkiss/NeMo that referenced this pull request Mar 13, 2021
… pull down 1.8.0. Lightning-AI/pytorch-lightning#6168

Signed-off-by: Tibor Kiss <kiss.tibor@gmail.com>
tiborkiss added a commit to tiborkiss/NeMo that referenced this pull request Mar 15, 2021
… pull down 1.8.0. Lightning-AI/pytorch-lightning#6168

Signed-off-by: Tibor Kiss <kiss.tibor@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working duplicate This issue or pull request already exists
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Newer torchtext (>= nightly 2021-02-19) breaks PyTorch Lightning
4 participants