Skip to content

Conversation

@nathanpainchaud
Copy link
Contributor

@nathanpainchaud nathanpainchaud commented Aug 19, 2025

Fixes #9811

The fix proposed in #10404 was a step in the right direction by trying to import from the version installed by the user. However, as described in this comment, Lighting also installs a version of pytorch_lightning. So it should be checked second, as it will be available whether the user originally installed lightning or pytorch_lightning.

This is what the current PR does.

I tried implementing a test based on the example provided in #9811. However, I don't know how to dynamically (un)install packages (in this case lightning) in the current environment. If you want a test for the issue, and know of a way to install packages at runtime, please point it to me and I'll try to adapt my existing local tests.


EDIT: For reference, here are tables describing the compatibility between the Lightning package version installed and the import style:

Before this PR

how you import PL pip install lightning pip install pytorch-lightning
import lightning Runtime issues (similar to #9811) Crash on import
import pytorch_lightning Fully supported Fully supported

After this PR

how you import PL pip install lightning pip install pytorch-lightning
import lightning Fully supported Crash on import
import pytorch_lightning Runtime issues (similar to #9811) Fully supported

@codecov
Copy link

codecov bot commented Aug 21, 2025

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 85.16%. Comparing base (c211214) to head (82514c9).
⚠️ Report is 139 commits behind head on master.

Additional details and impacted files
@@            Coverage Diff             @@
##           master   #10417      +/-   ##
==========================================
- Coverage   86.11%   85.16%   -0.95%     
==========================================
  Files         496      509      +13     
  Lines       33655    35947    +2292     
==========================================
+ Hits        28981    30615    +1634     
- Misses       4674     5332     +658     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

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.

@nathanpainchaud Thank you very much again for sending this fix. Could you document this in your PR description so that whenever someone sees this issue, we can refer to this PR description?

Before this PR

how you import PL pip install lightning pip install pytorch-lightning
import lightning
import pytorch_lightning

After this PR

how you import PL pip install lightning pip install pytorch-lightning
import lightning
import pytorch_lightning

@akihironitta akihironitta enabled auto-merge (squash) September 5, 2025 18:45
@akihironitta akihironitta merged commit c139343 into pyg-team:master Sep 5, 2025
16 checks passed
@nathanpainchaud
Copy link
Contributor Author

Thank you very much again for sending this fix.

Thank you for finally merging it 🙂

Could you document this in your PR description so that whenever someone sees this issue, we can refer to this PR description?

@akihironitta Done. Let me know if you want to change the way I documented thigs.

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.

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

2 participants