classification
Title: digit check logic can be replaced by Py_ISDIGIT on prepare_s
Type: Stage: resolved
Components: Versions: Python 3.9, Python 3.8, Python 3.7
process
Status: closed Resolution: wont fix
Dependencies: Superseder:
Assigned To: corona10 Nosy List: corona10, mark.dickinson, meador.inge, serhiy.storchaka
Priority: normal Keywords: patch

Created on 2019-10-14 18:30 by corona10, last changed 2019-10-16 07:16 by corona10. This issue is now closed.

Pull Requests
URL Status Linked Edit
PR 16787 closed corona10, 2019-10-14 18:31
Messages (7)
msg354643 - (view) Author: Dong-hee Na (corona10) * (Python committer) Date: 2019-10-14 18:30
digit check logic can be replaced by Py_ISDIGIT on prepare_s.
msg354715 - (view) Author: Serhiy Storchaka (serhiy.storchaka) * (Python committer) Date: 2019-10-15 11:58
What is the benefit of such change?
msg354720 - (view) Author: Dong-hee Na (corona10) * (Python committer) Date: 2019-10-15 12:56
> What is the benefit of such change?


I think that if an API is implemented for a specific purpose,
it should be used. This will help us to maintain code quality.

Especially in the case of pointed out logics,
since Py_ISDIGIT is exactly what we want to do, 
By replacing this logic, we can reduce unnecessary mistakes when someone modifies the logic.

This is, of course, my personal opinion.
I think it may be different from the policies of the core developers.

Thank you
msg354736 - (view) Author: Serhiy Storchaka (serhiy.storchaka) * (Python committer) Date: 2019-10-15 15:57
It was added as a replacement of locale aware isdigit(). It was initially used only for implementing bytes.isdigit() and in PyOS_ascii_strtod().

isdigit() is not used in Modules/_struct.c, so there is nothing to replace with Py_ISDIGIT(). Currently hardcoded '0' and '9' are used in more places than Py_ISDIGIT(). Rewriting all this code just because we have Py_ISDIGIT() is a code churn.
msg354739 - (view) Author: Serhiy Storchaka (serhiy.storchaka) * (Python committer) Date: 2019-10-15 16:03
I left this on to the struct module maintainers.
msg354771 - (view) Author: Mark Dickinson (mark.dickinson) * (Python committer) Date: 2019-10-16 06:28
> Rewriting all this code just because we have Py_ISDIGIT() is a code churn.

I'm inclined to agree: the current code works and is efficient; there's no strong reason to change it.
msg354776 - (view) Author: Dong-hee Na (corona10) * (Python committer) Date: 2019-10-16 07:01
> I'm inclined to agree: the current code works and is efficient; there's no strong reason to change it.

Okay, then I will close the PR.
I respect core developer's opinions.
History
Date User Action Args
2019-10-16 07:16:30corona10setstatus: open -> closed
resolution: wont fix
stage: patch review -> resolved
2019-10-16 07:01:40corona10setmessages: + msg354776
2019-10-16 06:28:01mark.dickinsonsetmessages: + msg354771
2019-10-15 16:03:55serhiy.storchakasetnosy: + mark.dickinson, meador.inge
messages: + msg354739
2019-10-15 15:57:47serhiy.storchakasetmessages: + msg354736
2019-10-15 12:56:13corona10setmessages: + msg354720
2019-10-15 11:58:51serhiy.storchakasetnosy: + serhiy.storchaka
messages: + msg354715
2019-10-14 18:34:01corona10setversions: + Python 3.7, Python 3.8, Python 3.9
2019-10-14 18:31:49corona10setkeywords: + patch
stage: patch review
pull_requests: + pull_request16347
2019-10-14 18:30:34corona10create