-
-
Notifications
You must be signed in to change notification settings - Fork 32.5k
gh-73487: Convert _decimal
to use Argument Clinic (part 1)
#137606
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 and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Conversation
812708a
to
439380e
Compare
This comment has been minimized.
This comment has been minimized.
Is the
It looks worse than it actually is, only ~400 lines diff in |
As I said, it's more or less a mechanical change. No changes in signatures, docstrings are copied from docstrings.h (though, AC enforces to have PEP summary line, most decimal docstrings don't follow this - that changed). There should be a minor speedup from using METH_FASTCALL (see issue thread for examples, I'll do benchmarks later, maybe add news). I'm planning to use also METH_METHOD where possible, but in a separate patch.
Different types, then method functions? I don't see how to make such split uniform. |
This comment was marked as outdated.
This comment was marked as outdated.
_decimal
to use Argument Clinic
Co-authored-by: Adam Turner <9087854+AA-Turner@users.noreply.github.com>
This comment was marked as resolved.
This comment was marked as resolved.
This comment was marked as resolved.
This comment was marked as resolved.
Yes. I'm happy to look at the changes, just in a future PR instead of this one. |
This reverts commit 354d8db.
This comment was marked as resolved.
This comment was marked as resolved.
_decimal
to use Argument Clinic_decimal
to use Argument Clinic (part 1)
This comment was marked as outdated.
This comment was marked as outdated.
Ok, I did some final updates an added news. @AA-Turner, please review. I asked also @rhettinger and @serhiy-storchaka for review, as previous module maintainer was against AC change (though, I think he was fine with using METH_FASTCALL). Simple benchmark:
# bench.py
import pyperf
from decimal import Decimal as D
from decimal import ROUND_HALF_EVEN as rnd, getcontext
a, b = map(D, [1, 2])
ctx = getcontext()
runner = pyperf.Runner()
runner.bench_func('a.copy_sign(b)', a.copy_sign, b)
runner.bench_func('a.to_integral(rnd, ctx)', a.to_integral, rnd, ctx) pydoc diff: --- ref.txt 2025-08-11 11:04:08.585913041 +0300
+++ patch.txt 2025-08-11 10:46:13.043516864 +0300
@@ -567,19 +567,19 @@
| Return the adjusted exponent of the number. Defined as exp + digits - 1.
|
| as_integer_ratio(self, /)
- | Decimal.as_integer_ratio() -> (int, int)
+ | Return a pair of integers whose ratio is exactly equal to the original.
|
- | Return a pair of integers, whose ratio is exactly equal to the original
- | Decimal and with a positive denominator. The ratio is in lowest terms.
+ | The ratio is in lowest terms and with a positive denominator.
| Raise OverflowError on infinities and a ValueError on NaNs.
|
| as_tuple(self, /)
| Return a tuple representation of the number.
|
| canonical(self, /)
- | Return the canonical encoding of the argument. Currently, the encoding
- | of a Decimal instance is always canonical, so this operation returns its
- | argument unchanged.
+ | Return the canonical encoding of the argument.
+ |
+ | Currently, the encoding of a Decimal instance is always canonical, so this
+ | operation returns its argument unchanged.
|
| compare(self, /, other, context=None)
| Compare self to other. Return a decimal value:
@@ -626,16 +626,21 @@
| Return self.
|
| copy_abs(self, /)
- | Return the absolute value of the argument. This operation is unaffected by
- | context and is quiet: no flags are changed and no rounding is performed.
+ | Return the absolute value of the argument.
+ |
+ | This operation is unaffected by context and is quiet: no flags are changed and
+ | no rounding is performed.
|
| copy_negate(self, /)
- | Return the negation of the argument. This operation is unaffected by context
- | and is quiet: no flags are changed and no rounding is performed.
+ | Return the negation of the argument.
+ |
+ | This operation is unaffected by context and is quiet: no flags are changed and
+ | no rounding is performed.
|
| copy_sign(self, /, other, context=None)
- | Return a copy of the first operand with the sign set to be the same as the
- | sign of the second operand. For example:
+ | Return a copy of *self* with the sign set to be the same as the sign of *other*.
+ |
+ | For example:
|
| >>> Decimal('2.3').copy_sign(Decimal('-1.5'))
| Decimal('-2.3')
@@ -763,8 +768,9 @@
| to the equivalent value Decimal('32.1').
|
| number_class(self, /, context=None)
- | Return a string describing the class of the operand. The returned value
- | is one of the following ten strings:
+ | Return a string describing the class of the operand.
+ |
+ | The returned value is one of the following ten strings:
|
| * '-Infinity', indicating that the operand is negative infinity.
| * '-Normal', indicating that the operand is a negative normal number.
@@ -778,8 +784,7 @@
| * 'sNaN', indicating that the operand is a signaling NaN.
|
| quantize(self, /, exp, rounding=None, context=None)
- | Return a value equal to the first operand after rounding and having the
- | exponent of the second operand.
+ | Return a value equal to *self* after rounding, with the exponent of *other*.
|
| >>> Decimal('1.41421356').quantize(Decimal('1.000'))
| Decimal('1.414')
@@ -798,7 +803,9 @@
| argument is given, the rounding mode of the current thread's context is used.
|
| radix(self, /)
- | Return Decimal(10), the radix (base) in which the Decimal class does
+ | Return Decimal(10).
+ |
+ | This is the radix (base) in which the Decimal class does
| all its arithmetic. Included for compatibility with the specification.
|
| remainder_near(self, /, other, context=None)
@@ -846,34 +853,41 @@
| correctly rounded using the ROUND_HALF_EVEN rounding mode.
|
| to_eng_string(self, /, context=None)
- | Convert to an engineering-type string. Engineering notation has an exponent
- | which is a multiple of 3, so there are up to 3 digits left of the decimal
- | place. For example, Decimal('123E+1') is converted to Decimal('1.23E+3').
+ | Convert to an engineering-type string.
+ |
+ | Engineering notation has an exponent which is a multiple of 3, so there are up
+ | to 3 digits left of the decimal place. For example, Decimal('123E+1') is
+ | converted to Decimal('1.23E+3').
|
| The value of context.capitals determines whether the exponent sign is lower
| or upper case. Otherwise, the context does not affect the operation.
|
| to_integral(self, /, rounding=None, context=None)
- | Identical to the to_integral_value() method. The to_integral() name has been
- | kept for compatibility with older versions.
+ | Identical to the to_integral_value() method.
+ |
+ | The to_integral() name has been kept for compatibility with older versions.
|
| to_integral_exact(self, /, rounding=None, context=None)
- | Round to the nearest integer, signaling Inexact or Rounded as appropriate if
+ | Round to the nearest integer.
+ |
+ | Decimal.to_integral_exact() signals Inexact or Rounded as appropriate if
| rounding occurs. The rounding mode is determined by the rounding parameter
| if given, else by the given context. If neither parameter is given, then the
| rounding mode of the current default context is used.
|
| to_integral_value(self, /, rounding=None, context=None)
- | Round to the nearest integer without signaling Inexact or Rounded. The
- | rounding mode is determined by the rounding parameter if given, else by
- | the given context. If neither parameter is given, then the rounding mode
- | of the current default context is used.
+ | Round to the nearest integer without signaling Inexact or Rounded.
+ |
+ | The rounding mode is determined by the rounding parameter if given, else by
+ | the given context. If neither parameter is given, then the rounding mode of
+ | the current default context is used.
|
| ----------------------------------------------------------------------
| Class methods defined here:
|
| from_float(f, /)
| Class method that converts a float to a decimal number, exactly.
+ |
| Since 0.1 is not exactly representable in binary floating point,
| Decimal.from_float(0.1) is not the same as Decimal('0.1').
| |
FYI: #137637 finish conversion. |
Reading through Stefan's previous comments, they are primarily on readability and performance. I think readability improves here -- the file does become longer, but we now have in-line documentation for each function, like you would in a Python file. Most of the rest of the stdlib extension modules also use AC, so it makes the contributor experience more consistent. Finally, having escaping automatically handled for docstrings is always nice. Performance seems a clear win, per the benchmarks @skirpichev posted. Argument parsing code does not take long, but some of the functions in I propose to merge this later in the week if Serhiy and Raymond don't have any major objections, then we can continue with @skirpichev's part 2 PR. A |
I would appreciate if #137624 can be merged first. It seems, Serhiy has only few nitpicks on that pr. FYI: I cleaned some comments as redundant. |
Uh oh!
There was an error while loading. Please reload this page.