python / cpython

The Python programming language
https://www.python.org
Other
63.41k stars 30.36k forks source link

Add deprecation of float limits for resource to documentation #87267

Open f2eea162-6427-4d3b-8889-5a9b9c78e6eb opened 3 years ago

f2eea162-6427-4d3b-8889-5a9b9c78e6eb commented 3 years ago
BPO 43101
Nosy @mdickinson, @serhiy-storchaka

Note: these values reflect the state of the issue at the time it was migrated and might not reflect the current state.

Show more details

GitHub fields: ```python assignee = None closed_at = None created_at = labels = ['3.8', '3.9', '3.10'] title = 'Add deprecation of float limits for resource to documentation' updated_at = user = 'https://bugs.python.org/nafur' ``` bugs.python.org fields: ```python activity = actor = 'mark.dickinson' assignee = 'none' closed = False closed_date = None closer = None components = [] creation = creator = 'nafur' dependencies = [] files = [] hgrepos = [] issue_num = 43101 keywords = [] message_count = 5.0 messages = ['386139', '386636', '386637', '386643', '386644'] nosy_count = 3.0 nosy_names = ['mark.dickinson', 'serhiy.storchaka', 'nafur'] pr_nums = [] priority = 'normal' resolution = None stage = None status = 'open' superseder = None type = None url = 'https://bugs.python.org/issue43101' versions = ['Python 3.8', 'Python 3.9', 'Python 3.10'] ```

f2eea162-6427-4d3b-8889-5a9b9c78e6eb commented 3 years ago

While the documentation always (as in: at least since 3.5) required to pass "a tuple (soft, hard) of two integers" to resource.setrlimit(), passing floats worked just fine until 3.9. This behavior was deprecated in 3.8 and removed in 3.10.

I see that the implementation was merely fixed to do what the documentation says. Nevertheless, I think this change of behavior should be mentioned in the documentation.

In my use-case for resource (within preexec_fn of subprocess.Popen), this deprecation warning only showed up in 3.9 for some reason (as you can see here: https://github.com/nafur/python-playground/runs/1814573503) and is now hidden by a generic "subprocess.SubprocessError: Exception occurred in preexec_fn." message. A hint in the documentation would have helped significantly...

serhiy-storchaka commented 3 years ago

This change was indirectly documented in What's New for 3.8 (deprecation) https://docs.python.org/3.8/whatsnew/3.8.html#build-and-c-api-changes:

""" Functions that convert Python number to C integer like PyLong_AsLong() and argument parsing functions like PyArgParseTuple() with integer converting format units like 'i' will now use the \_index() special method instead of __int(), if available. The deprecation warning will be emitted for objects with the __int() method but without the __index() method (like Decimal and Fraction). PyNumberCheck() will now return 1 for objects implementing \_index(). PyNumber_Long(), PyNumber_Float() and PyFloatAsDouble() also now use the \_index() method if available. """

and for 3.10 (removing) https://docs.python.org/3.10/whatsnew/3.10.html#other-language-changes:

""" Builtin and extension functions that take integer arguments no longer accept Decimals, Fractions and other objects that can be converted to integers only with a loss (e.g. that have the __int() method but do not have the __index() method). """

It was impractical to document it for every affected function, because there may be many tens of such functions in the stdlib, and it is difficult to find all functions which directly or indirectly use PyLong_AsLong() and similar C API. In any case accepting non-integer numbers was not intentional.

mdickinson commented 3 years ago

Agreed with Serhiy that it's not practical to document all the affected functions.

f2eea162-6427-4d3b-8889-5a9b9c78e6eb commented 3 years ago

Hm, I see.

For methods that do not intercept exceptions or run code in a different process or alike, the deprecation warning and the TypeError should be sufficient.

Given that identifying this issue can be particularly nasty for setrlimit (if used within preexec_fn), could we document this particular affected function anyway?

mdickinson commented 3 years ago

Maybe the better fix is to have better reporting of the preexec_fn error, so that it's not entirely hidden by the SubprocessError? On the other hand, given that preexec_fn may be going away entirely soon (see bpo-38435), that may not be worth it.