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

Improve error message #1556

Merged
merged 6 commits into from
Mar 29, 2023
Merged

Conversation

anoukvlug
Copy link
Collaborator

This PR addresses issue #1555, by improving the error message that is being raised for the following error ModuleNotFoundError: No module named 'shapely.io'.

@TimoRoth
Copy link
Member

Is the new notebook part of this PR intentionally?
Also, the text of the error message can change depending on the python version. And maybe even system language?
Much better to check the name property of the exception instead.

@anoukvlug
Copy link
Collaborator Author

Yes, that does make sense. However I don't think I fully understand you suggestion. What do you mean with name property?

Thanks for noticing the notebook, that was not supposed to be included in this PR.

@TimoRoth
Copy link
Member

ModuleNotFoundError inherits the name property from the ImportError Exception class.
So you can just get the module that wasn't found from the name property:
if err.name == "shapely.io":

You'll also most definitely want to add an else-Branch to that if that re-raises the exception, otherwise this could hide other errors and continue despite the loading having failed.

@anoukvlug
Copy link
Collaborator Author

Thanks for the explanation @TimoRoth! Is the latest commit what you had in mind?

@TimoRoth
Copy link
Member

To re-raise an exception, just call raise without any arguments. Otherwise you might obscure where the error actually came from.

One more issue I could see is that by creating a new ModuleNotFoundError without the name/path argument set, you might be breaking conventions.

What you could do instead:

                if err.name == "shapely.io":
                    err.msg = "You need shapely version 2.0 or higher for this to work."
                raise

@anoukvlug
Copy link
Collaborator Author

Thanks for the coding lesson. That looks indeed better :)

@fmaussion
Copy link
Member

Thanks!

@fmaussion fmaussion merged commit e87674a into OGGM:master Mar 29, 2023
@anoukvlug anoukvlug deleted the improve_error_message branch March 29, 2023 14:35
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.

None yet

3 participants