Skip to content

MAINT: cleanup use of sys.exc_info#15248

Merged
charris merged 1 commit intonumpy:masterfrom
eric-wieser:avoid-exc_info
Jan 6, 2020
Merged

MAINT: cleanup use of sys.exc_info#15248
charris merged 1 commit intonumpy:masterfrom
eric-wieser:avoid-exc_info

Conversation

@eric-wieser
Copy link
Member

This code originates from python 2.6, before there was an as clause in except.

This removes all callers of numpy.distutils.compat.get_exception.

This code originates from python 2.6, before there was an `as` clause in `except`.

This removes all callers of `numpy.distutils.compat.get_exception`.
except (DistutilsExecError, CompileError):
str(get_exception())
except (DistutilsExecError, CompileError) as e:
str(e)
Copy link
Member

@charris charris Jan 5, 2020

Choose a reason for hiding this comment

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

Does this actually do anything.

Copy link
Member Author

Choose a reason for hiding this comment

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

I mean, in principle it could if someone declares:

class WeirdException(CompileError):
    def __str__(self):
       print("Yes this does something")
       return CompilerError.__str__(self)

I was trying to keep the changes in this patch strictly to removing workarounds except E as e:, rather than evaluating whether e was used sensibly in the first place.

Copy link
Member

@charris charris Jan 6, 2020

Choose a reason for hiding this comment

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

Ha, it was intended to fix an unused variable warning in #12448.

-           msg = str(get_exception())
+           str(get_exception())

Copy link
Member

Choose a reason for hiding this comment

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

My guess is that the fixer thought there might be a side effect in the function call.

Copy link
Member Author

Choose a reason for hiding this comment

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

Thanks for the detective work - I think I'd still rather leave that to a follow-up PR - searching for str(e) probably finds a large amount of stuff that can be better written with exception chaining.

Copy link
Member

Choose a reason for hiding this comment

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

Flake8 doesn't like that file :) But it doesn't complain about unused variables. I think we can improve a lot of messages with f-strings. We could do that now that Python 3.5 has been dropped, but probably better to wait for the next release.

@charris charris merged commit ba81c42 into numpy:master Jan 6, 2020
@charris
Copy link
Member

charris commented Jan 6, 2020

Thanks Eric.

seberg pushed a commit that referenced this pull request Apr 11, 2023
This is similar to #15248, removing the remaining usages of sys.exc_info() that are no longer necessary.
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.

2 participants