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

Migrate old pytorch-lightning imports to modern lightning imports #9887

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

nathanpainchaud
Copy link

Fixes #9811.

As the title says. PyTorch Lightning as been recommending a new import style (import Lightning as L rather than import pytorch_lightning as pl) since early 2023 now.

The old style was/is still supported for backwards compatibility, but both styles cannot be used at the same time (see here). Since PyG uses the old style, this means that projects using PyG are stuck with the old style. Most importantly, it can cause clashes when projects rely on other dependencies that use the modern import style (see #9811).

Therefore, I would suggest that PyG migrates to the new style, like other projects have done (e.g. Optuna here).

Since the task didn't seem to massive, I've migrated over all of the Lightning that I found across the project, and standardized the import styles between examples and tests (to use the style recommended in Lightning's own docs).

Copy link
Member

@akihironitta akihironitta left a comment

Choose a reason for hiding this comment

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

Hi @nathanpainchaud, thanks for working on this with the PR description :) Given comments in the linked issues, we should definitely do something about this.

For the reason stated in #9235 (reply in thread), could we:

  • keep the import statements in examples as they are, and
  • support both pytorch_lightning and lightning names in torch_geometric/ to avoid a breaking change?

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.

ValueError("Expected a parent") when using new Lightning imports and PyG Lightning DataModule wrappers
2 participants