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

[refactor] Use ModuleNotFoundError instead of ImportError for consistency #9556

Closed
akihironitta opened this issue Sep 16, 2021 · 3 comments · Fixed by #9867
Closed

[refactor] Use ModuleNotFoundError instead of ImportError for consistency #9556

akihironitta opened this issue Sep 16, 2021 · 3 comments · Fixed by #9867
Assignees
Labels

Comments

@akihironitta
Copy link
Contributor

akihironitta commented Sep 16, 2021

Proposed refactoring or deprecation

[nitpicking] Both ModuleNotFoundError and ImportError are used in the codebase when users try to use extensions that require optional packages without them installed, but only one of the exceptions should be used for consistency.

Do we use these exceptions differently depending on the situation? If not, we should consolidate them.

Occurences in the codebase
$ egrep -rn "raise (ModuleNotFoundError|ImportError)" pytorch_lightning -B1
pytorch_lightning/loggers/mlflow.py-119-        if mlflow is None:
pytorch_lightning/loggers/mlflow.py:120:            raise ImportError(
--
pytorch_lightning/loggers/test_tube.py-108-        if Experiment is None:
pytorch_lightning/loggers/test_tube.py:109:            raise ImportError(
--
pytorch_lightning/loggers/wandb.py-116-        if wandb is None:
pytorch_lightning/loggers/wandb.py:117:            raise ImportError(
--
pytorch_lightning/loggers/comet.py-143-        if comet_ml is None:
pytorch_lightning/loggers/comet.py:144:            raise ImportError(
--
pytorch_lightning/callbacks/progress/rich_progress.py-114-        if not _RICH_AVAILABLE:
pytorch_lightning/callbacks/progress/rich_progress.py:115:            raise ModuleNotFoundError(
--
pytorch_lightning/profiler/pytorch.py-107-        if not _KINETO_AVAILABLE:
pytorch_lightning/profiler/pytorch.py:108:            raise ModuleNotFoundError("You are trying to use `ScheduleWrapper` which require kineto install.")
--
pytorch_lightning/utilities/cli.py-47-        if not _JSONARGPARSE_AVAILABLE:
pytorch_lightning/utilities/cli.py:48:            raise ModuleNotFoundError(

Additional context

FYI, https://docs.python.org/3/library/exceptions.html#ImportError
ImportError

Raised when the import statement has troubles trying to load a module. Also raised when the “from list” in from ... import has a name that cannot be found.

The name and path attributes can be set using keyword-only arguments to the constructor. When set they represent the name of the module that was attempted to be imported and the path to any file which triggered the exception, respectively.

Changed in version 3.3: Added the name and path attributes.

ModuleNotFoundError

A subclass of ImportError which is raised by import when a module could not be located. It is also raised when None is found in sys.modules.
New in version 3.6.

@akihironitta akihironitta added refactor good first issue Good for newcomers and removed refactor labels Sep 16, 2021
@kaushikb11
Copy link
Contributor

Also to note, we are using MisconfigurationException at some places.

@bamblebam
Copy link
Contributor

ig import error would be the safer option since it covers the module not found error as well

@sidml
Copy link
Contributor

sidml commented Oct 5, 2021

@akihironitta
If anyone else is not working on this, I can make a PR fixing this.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants