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

pl version: examples/requirements.txt is single source of truth #6309

Merged
merged 1 commit into from
Aug 11, 2020

Conversation

stas00
Copy link
Contributor

@stas00 stas00 commented Aug 6, 2020

PL git master is unstable:

cd examples/text-classification
./run_pl.sh
   File "run_pl_glue.py", line 12, in <module>
    from lightning_base import BaseTransformer, add_generic_args, generic_train
  File "/mnt/nvme1/code/huggingface/transformers-master/examples/lightning_base.py", line 7, in <module>
    import pytorch_lightning as pl
  File "/home/stas/anaconda3/envs/main/lib/python3.7/site-packages/pytorch_lightning/__init__.py", line 76, in <module>
    __import__('pkg_resources').declare_namespace(__name__)
  File "/home/stas/anaconda3/envs/main/lib/python3.7/site-packages/pkg_resources/__init__.py", line 2301, in declare_namespace
    _handle_ns(packageName, path_item)
  File "/home/stas/anaconda3/envs/main/lib/python3.7/site-packages/pkg_resources/__init__.py", line 2234, in _handle_ns
    loader.load_module(packageName)
  File "/mnt/nvme1/code/github/00pytorch/pytorch-lightning/pytorch_lightning/__init__.py", line 56, in <module>
    from pytorch_lightning.core import LightningDataModule, LightningModule
ImportError: cannot import name 'LightningDataModule' from 'pytorch_lightning.core' (/home/stas/anaconda3/envs/main/lib/python3.7/site-packages/pytorch_lightning/core/__init__.py)

the scripts will now rely on:

grep pytorch-l examples/requirements.txt
pytorch-lightning==0.8.5

whenever the requirement removed by this PR was added (it also helps to add why it was added):

# Install newest ptl.
pip install -U git+http://github.com/PyTorchLightning/pytorch-lightning/

seems to no longer be needed - at least the code runs to completion.

@codecov
Copy link

codecov bot commented Aug 7, 2020

Codecov Report

Merging #6309 into master will increase coverage by 0.07%.
The diff coverage is n/a.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #6309      +/-   ##
==========================================
+ Coverage   79.44%   79.52%   +0.07%     
==========================================
  Files         148      148              
  Lines       27193    27193              
==========================================
+ Hits        21604    21625      +21     
+ Misses       5589     5568      -21     
Impacted Files Coverage Δ
src/transformers/modeling_tf_flaubert.py 24.53% <0.00%> (-63.20%) ⬇️
src/transformers/tokenization_bart.py 60.56% <0.00%> (-35.22%) ⬇️
src/transformers/generation_tf_utils.py 86.46% <0.00%> (+1.00%) ⬆️
src/transformers/tokenization_dpr.py 57.65% <0.00%> (+4.50%) ⬆️
src/transformers/modeling_tf_distilbert.py 97.41% <0.00%> (+32.94%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 175cd45...6337bdf. Read the comment docs.

Copy link
Member

@LysandreJik LysandreJik left a comment

Choose a reason for hiding this comment

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

This LGTM, thanks @stas00.

@sshleifer, what do you think of this?

@sshleifer
Copy link
Contributor

I am strongly in favor.

@sshleifer sshleifer changed the title current PL master breaks - let's stick to release versions pl version: examples/requirements.txt is single source of truth Aug 11, 2020
@sshleifer sshleifer merged commit 7c6a085 into huggingface:master Aug 11, 2020
@stas00 stas00 deleted the pl branch August 11, 2020 16:47
fabiocapsouza added a commit to fabiocapsouza/transformers that referenced this pull request Nov 15, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants