bpo-21720: Improve exception message of import() by berkerpeksag · Pull Request #4113 · python/cpython (original) (raw)

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 andprivacy statement. We’ll occasionally send you account related emails.

Already on GitHub?Sign in to your account

Conversation5 Commits3 Checks0 Files changed

Conversation

This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.Learn more about bidirectional Unicode characters

[ Show hidden characters]({{ revealButtonHref }})

berkerpeksag

@berkerpeksag

Exception message was unclear when the type of fromlist parameter is not str:

TypeError: hasattr(): attribute name must be string

@ncoghlan

CI runs under -bb, so the tests fail with BytesWarning rather than the expected exception. To avoid that, the type checking loop will need to be a separate loop before the if statement that checks for "*".

@berkerpeksag

@berkerpeksag

@berkerpeksag

Thank you for the review, @ncoghlan. I've fixed the BytesWarning issue.

ncoghlan

serhiy-storchaka

@@ -96,6 +96,20 @@ def test_blocked_fromlist(self):
fromlist=[SUBMOD_NAME.rpartition('.')[-1]])
self.assertEqual(cm.exception.name, SUBMOD_NAME)
def test_fromlist_invalid_type(self):
with self.assertRaises(TypeError) as cm:

Choose a reason for hiding this comment

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

Add an 'error' filter for BytesWarning.

serhiy-storchaka

"Sequence item 0 in 'from list' must be str, not 'bytes'"
)
with self.assertRaises(TypeError) as cm:
self.__import__('encodings', fromlist=['aliases', b'codecs'])

Choose a reason for hiding this comment

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

I think it is better to use 'utf_8' instead of 'codecs'. 'codecs' is just an outer module imported in 'encodings'. It could be imported as '_codecs' for example.

@ncoghlan

@serhiy-storchaka proposed an alternate solution in #4118 (adjusting the importlib behaviour back to more closely matching the behaviour of the old import system), and that seems like a better option to me since it minimises the overhead of the new check, while still avoiding the cryptic bytes warning.