Skip to content

gh-73487: Convert _decimal to use Argument Clinic (part 2) #137637

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

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

skirpichev
Copy link
Contributor

@skirpichev skirpichev commented Aug 11, 2025

@skirpichev

This comment was marked as outdated.

@skirpichev

This comment was marked as outdated.

@skirpichev skirpichev marked this pull request as ready for review August 13, 2025 15:26
@skirpichev skirpichev requested review from a team and erlend-aasland as code owners August 13, 2025 15:26
@AA-Turner
Copy link
Member

@skirpichev This PR is +7000/-2000, it's too big. Do you have any suggestions to split it up into parts?

A

@skirpichev
Copy link
Contributor Author

How about the first commit?

@vstinner
Copy link
Member

Same here. If you want me to review your PR, please split it into smaller PRs.

@skirpichev skirpichev force-pushed the ac-decimal/73487-pt2 branch from 6c8f61f to 1b7845e Compare August 13, 2025 17:07
@skirpichev
Copy link
Contributor Author

skirpichev commented Aug 13, 2025

Ok, I left only first commit. Is this still too much?

@vstinner
Copy link
Member

I would prefer a PR which would be 1/3 or 1/4 of that.

@skirpichev
Copy link
Contributor Author

@skirpichev This PR is +7000/-2000, it's too big. Do you have any suggestions to split it up into parts?

But most of this - auto-generated content. Relevant changes are in _decimal.c.

Here is the original version:

$ git diff --stat  master..bak2 Modules/_decimal/_decimal.c
 Modules/_decimal/_decimal.c | 4000 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++------------------------------
 1 file changed, 2890 insertions(+), 1110 deletions(-)

Here the current:

$ git diff --stat master Modules/_decimal/_decimal.c
 Modules/_decimal/_decimal.c | 1345 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++------------------------
 1 file changed, 1045 insertions(+), 300 deletions(-)

I would prefer a PR which would be 1/3 or 1/4 of that.

Hmm, It seems I'll have to redo all work. Ok. Though, this will take some time...

BTW, documentation diff even smaller:

--- ref.txt 2025-08-13 20:30:52.481919501 +0300
+++ patch.txt   2025-08-13 20:53:51.808385431 +0300
@@ -452,12 +452,14 @@
      |      True if self else False
      |
      |  __ceil__(self, /)
+     |      Return the ceiling as an Integral.
      |
      |  __complex__(self, /)
+     |      Convert this value to exact type complex.
      |
      |  __copy__(self, /)
      |
-     |  __deepcopy__(self, object, /)
+     |  __deepcopy__(self, memo, /)
      |
      |  __divmod__(self, value, /)
      |      Return divmod(self, value).
@@ -469,14 +471,13 @@
      |      float(self)
      |
      |  __floor__(self, /)
+     |      Return the floor as an Integral.
      |
      |  __floordiv__(self, value, /)
      |      Return self//value.
      |
-     |  __format__(...)
-     |      Default object formatter.
-     |
-     |      Return str(self) if format_spec is empty. Raise TypeError otherwise.
+     |  __format__(self, /, fmtarg, override=None)
+     |      Formats the Decimal according to fmtarg.
      |
      |  __ge__(self, value, /)
      |      Return self>=value.
@@ -524,7 +525,7 @@
      |      Return divmod(value, self).
      |
      |  __reduce__(self, /)
-     |      Helper for pickle.
+     |      Return state information for pickling.
      |
      |  __repr__(self, /)
      |      Return repr(self).
@@ -538,7 +539,8 @@
      |  __rmul__(self, value, /)
      |      Return value*self.
      |
-     |  __round__(...)
+     |  __round__(self, /, ndigits=None)
+     |      Return the Integral closest to self, rounding half toward even.
      |
      |  __rpow__(self, value, mod=None, /)
      |      Return pow(value, self, mod).
@@ -550,7 +552,7 @@
      |      Return value/self.
      |
      |  __sizeof__(self, /)
-     |      Size of object in memory, in bytes.
+     |      Returns size in memory, in bytes
      |
      |  __str__(self, /)
      |      Return str(self).
@@ -562,6 +564,7 @@
      |      Return self/value.
      |
      |  __trunc__(self, /)
+     |      Return the Integral closest to x between 0 and x.
      |
      |  adjusted(self, /)
      |      Return the adjusted exponent (exp + digits - 1) of the number.
@@ -582,19 +585,22 @@
      |      so this operation returns its argument unchanged.
      |
      |  compare(self, /, other, context=None)
-     |      Compare self to other.  Return a decimal value:
+     |      Compare self to other.
+     |
+     |      Return a decimal value:
      |
-     |      a or b is a NaN ==> Decimal('NaN')
-     |      a < b           ==> Decimal('-1')
-     |      a == b          ==> Decimal('0')
-     |      a > b           ==> Decimal('1')
+     |          a or b is a NaN ==> Decimal('NaN')
+     |          a < b           ==> Decimal('-1')
+     |          a == b          ==> Decimal('0')
+     |          a > b           ==> Decimal('1')
      |
      |  compare_signal(self, /, other, context=None)
      |      Identical to compare, except that all NaNs signal.
      |
      |  compare_total(self, /, other, context=None)
-     |      Compare two operands using their abstract representation rather than
-     |      their numerical value.  Similar to the compare() method, but the result
+     |      Compare two operands using their abstract representation.
+     |
+     |      Similar to the compare() method, but the result
      |      gives a total ordering on Decimal instances.  Two Decimal instances with
      |      the same numeric value but different representations compare unequal
      |      in this ordering:
@@ -602,25 +608,28 @@
      |          >>> Decimal('12.0').compare_total(Decimal('12'))
      |          Decimal('-1')
      |
-     |      Quiet and signaling NaNs are also included in the total ordering. The result
-     |      of this function is Decimal('0') if both operands have the same representation,
-     |      Decimal('-1') if the first operand is lower in the total order than the second,
-     |      and Decimal('1') if the first operand is higher in the total order than the
-     |      second operand. See the specification for details of the total order.
-     |
-     |      This operation is unaffected by context and is quiet: no flags are changed
-     |      and no rounding is performed. As an exception, the C version may raise
-     |      InvalidOperation if the second operand cannot be converted exactly.
+     |      Quiet and signaling NaNs are also included in the total ordering. The
+     |      result of this function is Decimal('0') if both operands have the same
+     |      representation, Decimal('-1') if the first operand is lower in the
+     |      total order than the second, and Decimal('1') if the first operand is
+     |      higher in the total order than the second operand. See the
+     |      specification for details of the total order.
+     |
+     |      This operation is unaffected by context and is quiet: no flags are
+     |      changed and no rounding is performed. As an exception, the C version
+     |      may raise InvalidOperation if the second operand cannot be converted
+     |      exactly.
      |
      |  compare_total_mag(self, /, other, context=None)
-     |      Compare two operands using their abstract representation rather than their
-     |      value as in compare_total(), but ignoring the sign of each operand.
+     |      As compare_total(), but ignores the sign of each operand.
      |
-     |      x.compare_total_mag(y) is equivalent to x.copy_abs().compare_total(y.copy_abs()).
+     |      x.compare_total_mag(y) is equivalent to
+     |      x.copy_abs().compare_total(y.copy_abs()).
      |
-     |      This operation is unaffected by context and is quiet: no flags are changed
-     |      and no rounding is performed. As an exception, the C version may raise
-     |      InvalidOperation if the second operand cannot be converted exactly.
+     |      This operation is unaffected by context and is quiet: no flags are
+     |      changed and no rounding is performed. As an exception, the C version
+     |      may raise InvalidOperation if the second operand cannot be converted
+     |      exactly.
      |
      |  conjugate(self, /)
      |      Return self.
@@ -651,71 +660,78 @@
      |      exactly.
      |
      |  exp(self, /, context=None)
-     |      Return the value of the (natural) exponential function e**x at the given
-     |      number.  The function always uses the ROUND_HALF_EVEN mode and the result
-     |      is correctly rounded.
+     |      Return the value of the (natural) exponential function e**x.
+     |
+     |      The function always uses the ROUND_HALF_EVEN mode and the result is
+     |      correctly rounded.
      |
      |  fma(self, /, other, third, context=None)
-     |      Fused multiply-add.  Return self*other+third with no rounding of the
-     |      intermediate product self*other.
+     |      Fused multiply-add.
+     |
+     |      Return self*other+third with no rounding of the intermediate product
+     |      self*other.
      |
      |          >>> Decimal(2).fma(3, 5)
      |          Decimal('11')
      |
      |  is_canonical(self, /)
-     |      Return True if the argument is canonical and False otherwise.  Currently,
-     |      a Decimal instance is always canonical, so this operation always returns
-     |      True.
+     |      Return True if the argument is canonical and False otherwise.
+     |
+     |      Currently, a Decimal instance is always canonical, so this operation
+     |      always returns True.
      |
      |  is_finite(self, /)
-     |      Return True if the argument is a finite number, and False if the argument
-     |      is infinite or a NaN.
+     |      Return True if the argument is a finite number, and False otherwise.
      |
      |  is_infinite(self, /)
-     |      Return True if the argument is either positive or negative infinity and
-     |      False otherwise.
+     |      Return True if the argument is infinite, and False otherwise.
      |
      |  is_nan(self, /)
-     |      Return True if the argument is a (quiet or signaling) NaN and False
-     |      otherwise.
+     |      Return True if the argument is a (quiet or signaling) NaN, else False.
      |
      |  is_normal(self, /, context=None)
-     |      Return True if the argument is a normal finite non-zero number with an
-     |      adjusted exponent greater than or equal to Emin. Return False if the
-     |      argument is zero, subnormal, infinite or a NaN.
+     |      Return True if the argument is a normal number and False otherwise.
+     |
+     |      Normal number is a finite nonzero number, which is not subnormal.
      |
      |  is_qnan(self, /)
      |      Return True if the argument is a quiet NaN, and False otherwise.
      |
      |  is_signed(self, /)
      |      Return True if the argument has a negative sign and False otherwise.
+     |
      |      Note that both zeros and NaNs can carry signs.
      |
      |  is_snan(self, /)
      |      Return True if the argument is a signaling NaN and False otherwise.
      |
      |  is_subnormal(self, /, context=None)
-     |      Return True if the argument is subnormal, and False otherwise. A number is
-     |      subnormal if it is non-zero, finite, and has an adjusted exponent less
-     |      than Emin.
+     |      Return True if the argument is subnormal, and False otherwise.
+     |
+     |      A number is subnormal if it is non-zero, finite, and has an adjusted
+     |      exponent less than Emin.
      |
      |  is_zero(self, /)
-     |      Return True if the argument is a (positive or negative) zero and False
-     |      otherwise.
+     |      Return True if the argument is a zero and False otherwise.
      |
      |  ln(self, /, context=None)
-     |      Return the natural (base e) logarithm of the operand. The function always
-     |      uses the ROUND_HALF_EVEN mode and the result is correctly rounded.
+     |      Return the natural (base e) logarithm of the operand.
+     |
+     |      The function always uses the ROUND_HALF_EVEN mode and the result is
+     |      correctly rounded.
      |
      |  log10(self, /, context=None)
-     |      Return the base ten logarithm of the operand. The function always uses the
-     |      ROUND_HALF_EVEN mode and the result is correctly rounded.
+     |      Return the base ten logarithm of the operand.
+     |
+     |      The function always uses the ROUND_HALF_EVEN mode and the result is
+     |      correctly rounded.
      |
      |  logb(self, /, context=None)
-     |      For a non-zero number, return the adjusted exponent of the operand as a
-     |      Decimal instance.  If the operand is a zero, then Decimal('-Infinity') is
-     |      returned and the DivisionByZero condition is raised. If the operand is
-     |      an infinity then Decimal('Infinity') is returned.
+     |      Return the adjusted exponent of the operand as a Decimal instance.
+     |
+     |      If the operand is a zero, then Decimal('-Infinity') is returned and the
+     |      DivisionByZero condition is raised. If the operand is an infinity then
+     |      Decimal('Infinity') is returned.
      |
      |  logical_and(self, /, other, context=None)
      |      Return the digit-wise 'and' of the two (logical) operands.
@@ -727,46 +743,47 @@
      |      Return the digit-wise 'or' of the two (logical) operands.
      |
      |  logical_xor(self, /, other, context=None)
-     |      Return the digit-wise 'exclusive or' of the two (logical) operands.
+     |      Return the digit-wise 'xor' of the two (logical) operands.
      |
      |  max(self, /, other, context=None)
-     |      Maximum of self and other.  If one operand is a quiet NaN and the other is
-     |      numeric, the numeric operand is returned.
+     |      Maximum of self and other.
+     |
+     |      If one operand is a quiet NaN and the other is numeric, the numeric
+     |      operand is returned.
      |
      |  max_mag(self, /, other, context=None)
-     |      Similar to the max() method, but the comparison is done using the absolute
-     |      values of the operands.
+     |      As the max() method, but compares the absolute values of the operands.
      |
      |  min(self, /, other, context=None)
-     |      Minimum of self and other. If one operand is a quiet NaN and the other is
-     |      numeric, the numeric operand is returned.
+     |      Minimum of self and other.
+     |
+     |      If one operand is a quiet NaN and the other is numeric, the numeric
+     |      operand is returned.
      |
      |  min_mag(self, /, other, context=None)
-     |      Similar to the min() method, but the comparison is done using the absolute
-     |      values of the operands.
+     |      As the min() method, but compares the absolute values of the operands.
      |
      |  next_minus(self, /, context=None)
-     |      Return the largest number representable in the given context (or in the
-     |      current default context if no context is given) that is smaller than the
-     |      given operand.
+     |      Returns the largest representable number smaller than itself.
      |
      |  next_plus(self, /, context=None)
-     |      Return the smallest number representable in the given context (or in the
-     |      current default context if no context is given) that is larger than the
-     |      given operand.
+     |      Returns the smallest representable number larger than itself.
      |
      |  next_toward(self, /, other, context=None)
+     |      Returns the number closest to self, in the direction towards other.
+     |
      |      If the two operands are unequal, return the number closest to the first
      |      operand in the direction of the second operand.  If both operands are
      |      numerically equal, return a copy of the first operand with the sign set
      |      to be the same as the sign of the second operand.
      |
      |  normalize(self, /, context=None)
-     |      Normalize the number by stripping the rightmost trailing zeros and
-     |      converting any result equal to Decimal('0') to Decimal('0e0').  Used
-     |      for producing canonical values for members of an equivalence class.
-     |      For example, Decimal('32.100') and Decimal('0.321000e+2') both normalize
-     |      to the equivalent value Decimal('32.1').
+     |      Normalize the number by stripping trailing 0s
+     |
+     |      This also change anything equal to 0 to 0e0.  Used for producing
+     |      canonical values for members of an equivalence class.  For example,
+     |      Decimal('32.100') and Decimal('0.321000e+2') both normalize to
+     |      the equivalent value Decimal('32.1').
      |
      |  number_class(self, /, context=None)
      |      Return a string describing the class of the operand.
@@ -819,23 +836,25 @@
      |      all its arithmetic. Included for compatibility with the specification.
      |
      |  remainder_near(self, /, other, context=None)
-     |      Return the remainder from dividing self by other.  This differs from
-     |      self % other in that the sign of the remainder is chosen so as to minimize
-     |      its absolute value. More precisely, the return value is self - n * other
-     |      where n is the integer nearest to the exact value of self / other, and
-     |      if two integers are equally near then the even one is chosen.
+     |      Return the remainder from dividing self by other.
+     |
+     |      This differs from self % other in that the sign of the remainder is
+     |      chosen so as to minimize its absolute value. More precisely, the return
+     |      value is self - n * other where n is the integer nearest to the exact
+     |      value of self / other, and if two integers are equally near then the
+     |      even one is chosen.
      |
      |      If the result is zero then its sign will be the sign of self.
      |
      |  rotate(self, /, other, context=None)
-     |      Return the result of rotating the digits of the first operand by an amount
-     |      specified by the second operand.  The second operand must be an integer in
-     |      the range -precision through precision. The absolute value of the second
-     |      operand gives the number of places to rotate. If the second operand is
-     |      positive then rotation is to the left; otherwise rotation is to the right.
-     |      The coefficient of the first operand is padded on the left with zeros to
-     |      length precision if necessary. The sign and exponent of the first operand are
-     |      unchanged.
+     |      Returns a rotated copy of self's digits, value-of-other times.
+     |
+     |      The second operand must be an integer in the range -precision through
+     |      precision. The absolute value of the second operand gives the number of
+     |      places to rotate. If the second operand is positive then rotation is to
+     |      the left; otherwise rotation is to the right.  The coefficient of the
+     |      first operand is padded on the left with zeros to length precision if
+     |      necessary. The sign and exponent of the first operand are unchanged.
      |
      |  same_quantum(self, /, other, context=None)
      |      Test whether self and other have the same exponent or both are NaN.
@@ -846,22 +865,25 @@
      |      exactly.
      |
      |  scaleb(self, /, other, context=None)
-     |      Return the first operand with the exponent adjusted the second.  Equivalently,
-     |      return the first operand multiplied by 10**other. The second operand must be
-     |      an integer.
+     |      Return the first operand with the exponent adjusted the second.
+     |
+     |      Equivalently, return the first operand multiplied by 10**other. The
+     |      second operand must be an integer.
      |
      |  shift(self, /, other, context=None)
-     |      Return the result of shifting the digits of the first operand by an amount
-     |      specified by the second operand.  The second operand must be an integer in
-     |      the range -precision through precision. The absolute value of the second
-     |      operand gives the number of places to shift. If the second operand is
-     |      positive, then the shift is to the left; otherwise the shift is to the
-     |      right. Digits shifted into the coefficient are zeros. The sign and exponent
-     |      of the first operand are unchanged.
+     |      Returns a shifted copy of self's digits, value-of-other times.
+     |
+     |      The second operand must be an integer in the range -precision through
+     |      precision. The absolute value of the second operand gives the number
+     |      of places to shift. If the second operand is positive, then the shift
+     |      is to the left; otherwise the shift is to the right. Digits shifted
+     |      into the coefficient are zeros. The sign and exponent of the first
+     |      operand are unchanged.
      |
      |  sqrt(self, /, context=None)
-     |      Return the square root of the argument to full precision. The result is
-     |      correctly rounded using the ROUND_HALF_EVEN rounding mode.
+     |      Return the square root of the argument to full precision.
+     |
+     |      The result is correctly rounded using the ROUND_HALF_EVEN rounding mode.
      |
      |  to_eng_string(self, /, context=None)
      |      Convert to an engineering-type string.
@@ -1762,17 +1784,20 @@

 FUNCTIONS
     IEEEContext(bits, /)
-        Return a context object initialized to the proper values for one of the
-        IEEE interchange formats.  The argument must be a multiple of 32 and less
-        than IEEE_CONTEXT_MAX_BITS.
+        Return a context, initialized as one of the IEEE interchange formats.
+
+        The argument must be a multiple of 32 and less than
+        IEEE_CONTEXT_MAX_BITS.

     getcontext()
         Get the current default context.

     localcontext(ctx=None, **kwargs)
-        Return a context manager that will set the default context to a copy of ctx
-        on entry to the with-statement and restore the previous default context when
-        exiting the with-statement. If no context is specified, a copy of the current
+        Return a context manager for a copy of the supplied context.
+
+        That will set the default context to a copy of ctx on entry to the
+        with-statement and restore the previous default context when exiting
+        the with-statement. If no context is specified, a copy of the current
         default context is used.

     setcontext(context, /)

@serhiy-storchaka
Copy link
Member

I would prefer a single PR containing all changes of the same kind than 10 different PRs.

Different kinds of changes can be presented in different PRs.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants