classification
Title: Allow omission of imghdr.what file arg if bytes are given
Type: enhancement Stage: resolved
Components: Library (Lib) Versions: Python 3.10
process
Status: closed Resolution:
Dependencies: Superseder:
Assigned To: Nosy List: corona10, eamanu, terry.reedy
Priority: normal Keywords: patch

Created on 2020-09-09 03:56 by eamanu, last changed 2020-09-16 00:09 by eamanu. This issue is now closed.

Pull Requests
URL Status Linked Edit
PR 22166 closed eamanu, 2020-09-09 03:57
Messages (4)
msg376618 - (view) Author: Emmanuel Arias (eamanu) * Date: 2020-09-09 03:56
For the `what()` function, the `file` parameter is always needed.
In despite of that users can use `h` for send a bytes stream to 
detect the kind of the image, the `file` parameter is need.

Don't have sense ask for `file` parameter when this parameter will
not any effect on the result of the `what` function.
msg376632 - (view) Author: Dong-hee Na (corona10) * (Python committer) Date: 2020-09-09 12:31
IMHO, those use cases are already intended.
We can read such use case on test code but also documentation.

It will break legacy usage. I am -1 on change the API signature.
We should think deeply before changing this.
msg376758 - (view) Author: Terry J. Reedy (terry.reedy) * (Python committer) Date: 2020-09-12 01:18
The opening message confused me by using 'need' as 'currently required to call' rather than 'required to compute (test) and necessarily needed to call'; but after reading the code, I believe I understand and modified title to match.  More simply, one could call what(h=b'...') after the change instead of what(None, b'...').  This is the change in the tests (but the old form must still be tested also).

I don't see how the change would affect correct legacy use.  (The TypeError to ValueError change is wrong, and I don't understand the other exception change.)  However, what('') should get the same error as currently (see additional change in review).

However, what is very problemmatic is requiring 1 of 2 different arguments.  For range, the first parameter is really 'start_or_stop', an int either way, with the interpretation depending on the presence of a second.  This is a nuisance for understanding, but a convenience in usage.  But I think requiring an argument (usually) passed positionally or an argument that now has to be passed by keyword, with an arbitrary letter name, is worse and less justified.  So my current view is that this change should be rejected.
msg376969 - (view) Author: Emmanuel Arias (eamanu) * Date: 2020-09-16 00:09
> The opening message confused me by using 'need' as 'currently required to call' rather than 'required to compute (test) and necessarily needed to call'; but after reading the code, I believe I understand and modified title to match.

Sorry for my bad English, yes my idea was more likely your point of view. In the future I'll try to be more clear. 

> However, what is very problemmatic is requiring 1 of 2 different arguments.  For range, the first parameter is really 'start_or_stop', an int either way, with the interpretation depending on the presence of a second.  This is a nuisance for understanding, but a convenience in usage.  But I think requiring an argument (usually) passed positionally or an argument that now has to be passed by keyword, with an arbitrary letter name, is worse and less justified.  So my current view is that this change should be rejected.

My intention is try to avoid a call like `what(None, b'...')`. IMO call a function that start with None seems a little weird (from my point of view). Sincerely, I don't know if for the people it's ok use `what(None, b'...')` or not, probably not because haven't changed since 1997.

Reading your opinion it's more clear for me that this patch should be rejected. So, I'll close this issue and the PR.

Thanks for the feedback, very appreciate.
History
Date User Action Args
2020-09-16 00:09:31eamanusetstatus: open -> closed

messages: + msg376969
stage: patch review -> resolved
2020-09-12 01:18:37terry.reedysettitle: Little improve on imghdr library -> Allow omission of imghdr.what file arg if bytes are given
nosy: + terry.reedy

messages: + msg376758

type: behavior -> enhancement
2020-09-09 12:31:05corona10setnosy: + corona10
messages: + msg376632
2020-09-09 03:57:41eamanusetkeywords: + patch
stage: patch review
pull_requests: + pull_request21240
2020-09-09 03:56:25eamanucreate