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

Upgrades Torchvision, Lightning, Lightning Utilities and Gym[atari] and fixes imports. #988

Conversation

wilderrodrigues
Copy link

What does this PR do?

Currently, Lightning Bolts depends on a very old version of Lightning and on Torchvision 0.10. That hinders the development if one wants to stay up-to-date with the latest releases.

I would like to propose we update the versions of the packages and release a 0.7 version of Bolt

Fixes #962

Before submitting

  • Was this discussed/approved via a Github issue? (no need for typos and docs improvements)
  • Did you read the contributor guideline, Pull Request section?
  • Did you make sure your PR does only one thing, instead of bundling different changes together?
  • Did you make sure to update the documentation with your changes?
  • Did you write any new necessary tests? [not needed for typos/docs]
  • Did you verify new and existing tests pass locally with your changes?
  • If you made a notable change (that affects users), did you update the CHANGELOG?

PR review

  • Is this pull request ready for review? (if not, please submit in draft mode)

Anyone in the community is free to review the PR once the tests have passed.
If we didn't discuss your PR in Github issues there's a high chance it will not be merged.

Did you have fun?

Make sure you had fun coding 🙃

@wilderrodrigues
Copy link
Author

@Borda @akihironitta could you have a look at this, please?

@Borda
Copy link
Member

Borda commented Mar 6, 2023

will do later this week... 🐿️

@cdeepali
Copy link
Contributor

@wilderrodrigues, I saw following error while executing the tests:

ImportError while loading conftest '<mypath>/pytorch-lightning-bolts/tests/conftest.py'.
tests/conftest.py:8: in &lt;module&gt;
    from pytorch_lightning.trainer.connectors.signal_connector import SignalConnector
E   ImportError: cannot import name 'SignalConnector' from 'pytorch_lightning.trainer.connectors.signal_connector' 

For this could you add an update in your PR to use

from pytorch_lightning.trainer.connectors.signal_connector import _SignalConnector instead of from pytorch_lightning.trainer.connectors.signal_connector import SignalConnector in tests/conftest.py.

@wilderrodrigues
Copy link
Author

Thanks for the check and the code snippet, @cdeepali . Doing it now. Also, sorry for the delay, been loaded with day job work and couldn't get here before.

@zhutmost
Copy link

Hi. Will this pull request be merged in few days?
Our projects depend on DataModules in bolts, but it looks that the current nightly-edge bolts version is not compatible with Lightning 2.0.

CHANGELOG.md Outdated Show resolved Hide resolved
requirements.txt Outdated Show resolved Hide resolved
@Borda Borda force-pushed the master branch 2 times, most recently from afdc09a to c5137b4 Compare May 19, 2023 17:15
@mergify mergify bot removed the has conflicts label May 19, 2023
@Borda
Copy link
Member

Borda commented May 31, 2023

@wilderrodrigues, this is a very important contribution! 🐰 Can we help you?

@wilderrodrigues
Copy link
Author

@wilderrodrigues, this is a very important contribution! 🐰 Can we help you?

Hi @Borda ,

Been loaded with work lately.

I will rebase the branch today and update the PR. Also check what's going on with all those test failures.

@Borda
Copy link
Member

Borda commented Jun 23, 2023

I will rebase the branch today and update the PR. Also check what's going on with all those test failures.

just check if it would be easier to make several new PRs and addressing API updates for each dependency separately... :)

@Borda
Copy link
Member

Borda commented Jun 29, 2023

I'll be more than happy if you finish this upgrade, just as there are quite some collisions, let's start from the master and make each upgrade of a package in a separate PR 🐰

Just for the record:

@Borda Borda closed this Jun 29, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Cannot import name 'LightningLoggerBase' from 'pytorch_lightning.loggers'
4 participants