You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
I would like to suggest to implement a context manager class in order to handle optional imports better. With the context manager implemented, importing optional packages and deferring raising ModuleNotFoundError will look like:
withContextManager() ascm:
importtorchvision# doesn't raise `ModuleNotFoundError` here even if it fails.classSomeClass:
def__init__(self, ...):
cm.check() # raises `ModuleNotFoundError` if `import torchvision` failed.
...
Motivation
The suggested context manager reduces duplicate information in the codebase. Currently, there are redundant lines such as try: ... except ModuleNotFoundErorr: ..., _PACKAGE_AVAILABLE and raise ModuleNotFoundError(...).
Here is a minimal implementation of context manager and its behaviour.
Example
classContextManager:
def__init__(self):
self.exc=Nonedef__enter__(self):
returnselfdef__exit__(self, exc_type, exc_value, traceback):
self.exc=exc_type, exc_value, tracebackreturnTrue# Ignores exception if True and reraises exception if Falsedefcheck(self):
ifself.excisnotNone:
exc_type, exc_value, traceback=self.excraiseexc_type(exc_value).with_traceback(traceback)
# Here's how it behavesprint("Try to import.")
withContextManager() ascm:
importaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaprint("Tried to import.")
cm.check()
Try to import.
Tried to import.
Traceback (most recent call last):
File "main.py", line 22, in <module>
cm.check()
File "main.py", line 15, in check
raise exc_type(exc_value).with_traceback(traceback)
File "main.py", line 19, in <module>
import aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa
ModuleNotFoundError: No module named 'aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa'
Alternatives
As suggested in #338 (comment) by @Borda, we could define global variables like _TORCHVISION_AVAILABLE in pl_bolts/__init__.py, but in that way, we will have to add new variables like _NEW_PACKAGE_AVAILABLE as we add new optional packages to Bolts.
🚀 Feature
I would like to suggest to implement a context manager class in order to handle optional imports better. With the context manager implemented, importing optional packages and deferring raising
ModuleNotFoundError
will look like:Motivation
The suggested context manager reduces duplicate information in the codebase. Currently, there are redundant lines such as
try: ... except ModuleNotFoundErorr: ...
,_PACKAGE_AVAILABLE
andraise ModuleNotFoundError(...)
.Example
https://github.com/PyTorchLightning/pytorch-lightning-bolts/blob/4d254fde6112b21436003028d553a726bf7ea6ef/pl_bolts/datamodules/cifar10_datamodule.py#L12-L20
https://github.com/PyTorchLightning/pytorch-lightning-bolts/blob/4d254fde6112b21436003028d553a726bf7ea6ef/pl_bolts/datamodules/cifar10_datamodule.py#L86-L89
Pitch
Here is a minimal implementation of context manager and its behaviour.
Example
Alternatives
As suggested in #338 (comment) by @Borda, we could define global variables like
_TORCHVISION_AVAILABLE
inpl_bolts/__init__.py
, but in that way, we will have to add new variables like_NEW_PACKAGE_AVAILABLE
as we add new optional packages to Bolts.See also
Built-in Types - Context Manager Types
Stack Overflow - How to safely handle an exception inside a context manager
Other comments
We can implement the suggested context manager in PL and use it from Bolts. If it should be implemented in PL, could you transfer this issue to PL repo? GitHub Docs - Transferring an issue to another repository
Let me know if it sounds reasonable or too much engineering!
The text was updated successfully, but these errors were encountered: