Skip to content

bpo-41749: Little improve on imghdr #22166

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.

Sign up for GitHub

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

Closed
eamanu wants to merge 7 commits into from
Closed

bpo-41749: Little improve on imghdr #22166

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
e9a4f50
Little improve on imghdr
earias314 Sep 9, 2020
3d3d6c6
📜🤖 Added by blurb_it.
blurb-it[bot] Sep 9, 2020
de767b5
Update Tests
earias314 Sep 9, 2020
bf6a433
Merge branch 'little-improve-imghdr' of github.com:eamanu/cpython int…
earias314 Sep 9, 2020
e05389a
Change assert Raise for assertwarns
eamanu Sep 11, 2020
bdf5745
Update Lib/test/test_imghdr.py
eamanu Sep 13, 2020
0dee831
Update Lib/imghdr.py
eamanu Sep 13, 2020
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 4 additions & 1 deletion Lib/imghdr.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,10 +8,13 @@
# Recognize image headers #
#-------------------------#

def what(file, h=None):
def what(file='', h=None):

@terryjreedy terryjreedy Sep 12, 2020

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

To catch 'what()' properly, the default should be a sentinel that people do not pass. Before this add

Suggested change
def what(file='', h=None):
_none = object()
def what(file=_none, h=None):

f = None
try:
if h is None:
if file == '':

@terryjreedy terryjreedy Sep 12, 2020

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Suggested change
if file == '':
if file == _none:

raise TypeError("You need specify a str or PathLike for file, "
"or pass a byte stream as h parameter")
if isinstance(file, (str, PathLike)):
f = open(file, 'rb')
h = f.read(32)
Expand Down
10 changes: 5 additions & 5 deletions Lib/test/test_imghdr.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -49,8 +49,8 @@ def test_data(self):
self.assertEqual(imghdr.what(stream), expected)
with open(filename, 'rb') as stream:
data = stream.read()
self.assertEqual(imghdr.what(None, data), expected)
self.assertEqual(imghdr.what(None, bytearray(data)), expected)
self.assertEqual(imghdr.what(h=data), expected)
self.assertEqual(imghdr.what(h=bytearray(data)), expected)

def test_pathlike_filename(self):
for filename, expected in TEST_FILES:
Expand All @@ -64,7 +64,7 @@ def test_jumbo(h, file):
return 'ham'
imghdr.tests.append(test_jumbo)
self.addCleanup(imghdr.tests.pop)
self.assertEqual(imghdr.what(None, b'eggs'), 'ham')
self.assertEqual(imghdr.what(h=b'eggs'), 'ham')

def test_file_pos(self):
with open(TESTFN, 'wb') as stream:
Expand All @@ -83,7 +83,7 @@ def test_bad_args(self):
imghdr.what(None)
with self.assertRaises(TypeError):
imghdr.what(self.testfile, 1)
with self.assertRaises(AttributeError):
with self.assertRaises(BytesWarning):

@terryjreedy terryjreedy Sep 12, 2020

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Given that the signature change should not affect this call, I don't understand the error change.

imghdr.what(os.fsencode(self.testfile))
with open(self.testfile, 'rb') as f:
with self.assertRaises(AttributeError):
Expand All @@ -108,7 +108,7 @@ def test_string_data(self):
with self.assertRaises(TypeError):
imghdr.what(io.StringIO(data))
with self.assertRaises(TypeError):
imghdr.what(None, data)
imghdr.what(h=data)

def test_missing_file(self):
with self.assertRaises(FileNotFoundError):
Expand Down
1 change: 1 addition & 0 deletions Misc/NEWS.d/next/Library/2020-09-09-04-04-46.bpo-41749.xqeeeI.rst
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
Allow to programmer to don't use `file` parameter on `what` on imghdr library.

@terryjreedy terryjreedy Sep 12, 2020

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

This needs revision, but I won't bother unless we decide to apply change.