gh-102431: clarify constraints on arguments of logical_xxx methods

[skirpichev]

• Issue: Documentation of "decimal" library #102431

[skirpichev]

BTW, maybe it does make sense to add is_logical() method (there is _islogical() for pure-Python version) to test constraints for this type of decimal's. Probably, that could simplify documentation, esp. docstrings.

[rhettinger]

I would go farther an include the full text and examples used in Lib/_pydecimal.py:

    def logical_and(self, a, b):
        """Applies the logical operation 'and' between each operand's digits.

        The operands must be both logical numbers.

        >>> ExtendedContext.logical_and(Decimal('0'), Decimal('0'))
        Decimal('0')
        >>> ExtendedContext.logical_and(Decimal('0'), Decimal('1'))
        Decimal('0')
        >>> ExtendedContext.logical_and(Decimal('1'), Decimal('0'))
        Decimal('0')
        >>> ExtendedContext.logical_and(Decimal('1'), Decimal('1'))
        Decimal('1')
        >>> ExtendedContext.logical_and(Decimal('1100'), Decimal('1010'))
        Decimal('1000')
        >>> ExtendedContext.logical_and(Decimal('1111'), Decimal('10'))
        Decimal('10')
        >>> ExtendedContext.logical_and(110, 1101)
        Decimal('100')
        >>> ExtendedContext.logical_and(Decimal(110), 1101)
        Decimal('100')
        >>> ExtendedContext.logical_and(110, Decimal(1101))
        Decimal('100')
        """
        a = _convert_other(a, raiseit=True)
        return a.logical_and(b, context=self)

Anything less that this will just leave the user guessing about how to use these weird functions.

[skirpichev]

I would go farther an include the full text and examples used in Lib/_pydecimal.py:

Agreed. But there should be some formal description too, because "The operands must be both logical numbers." sounds cryptic even after examples... Or do you think it's ok? (The notion of "logical number" is well defined in the sphinx docs.)

I'll sync docstrings for C/Python implementations and add examples.

[skirpichev]

Ok, now docstrings of Context's methods are in sync.
But, maybe we could reduce the number of examples? E.g. in the logical_and case to:

>>> ExtendedContext.logical_and(Decimal('0'), Decimal('0'))
Decimal('0')
>>> ExtendedContext.logical_and(Decimal('0'), Decimal('1'))
Decimal('0')
>>> ExtendedContext.logical_and(Decimal('1'), Decimal('1'))
Decimal('1')
>>> ExtendedContext.logical_and(Decimal('1111'), Decimal('10'))
Decimal('10')

Basically, this shows everything: constraints on digits of operands, how it works digits-wise and how it works with coefficients of different length. I think it should reduce misunderstanding to minimum.

[skirpichev]

@rhettinger, does it make sense for you now?

[skirpichev]

CC @picnixz
CC @serhiy-storchaka

[picnixz]

I think this makes sense to have the inline definition of what a logical operand is but OTOH, one could say that the online docs should be sufficient so I'm neutral on this.

[serhiy-storchaka]

Sorry, I am not a Decimal expert and do not know why such weird operations exist at first place (perhaps it is just the part of some standard).

From practical point of view, I would just refer "logical integers" in the docstrings, and defined them in the module documentation. If you want to add examples in the docstrings and if there are examples in other docstring, I think that a single example per method should be enough. With multiple digits you can cover all cases by a single example. The rest can be added in the module documentation.

[skirpichev]

perhaps it is just the part of some standard

@serhiy-storchaka, sure. If you see something odd in the decimal module - IMO, it's coming from the IBM standard. JFR: https://speleotrove.com/decimal/damisc.html

If you want to add examples in the docstrings and if there are examples in other docstring, I think that a single example per method should be enough.

Current patch just sync Python (which has extensive examples for context functions) and C implementation. Do you think that we should remove (most) examples from pure-Python version?

[serhiy-storchaka]

Well, then we should keep examples. They are not just the documentation, but doctests.

But new additions are incorrect. "Both operands should have zero sign and exponent, and digits either 0 or 1." Operands can be integers, and integers don't have exponent or digits. Lets leave a more detailed explanation to the module documentation.

[skirpichev]

Operands can be integers, and integers don't have exponent or digits.

I doubt we should be more verbose here (docstrings!). And I don't think that sphinx docs needs to be expanded. Context docs already assume that integers are coerced to Decimal's: "Each Context method accepts a Python integer (an instance of int) anywhere that a Decimal instance is accepted."

[serhiy-storchaka]

I also do not think that we should be more verbose here.

[skirpichev]

CC @vstinner

[serhiy-storchaka]

I suggest to remove "both have zero sign and exponent, and digits either 0 or 1" from the docstrings. It is not accurate.

[skirpichev]

"both have zero sign and exponent, and digits either 0 or 1" from the docstrings. It is not accurate.

@serhiy-storchaka, why do you think so?

If you are about "Operands can be integers, and integers don't have exponent or digits." - no, they can't. Context's docs says, that integers are coerced to Decimal's. So, I think we can assume, that both operands are Decimal's.

[serhiy-storchaka]

But for Decimal methods this is incorrect.

Even if this is not incorrect for Context methods, it is verbose and repetitive. The current docstrings are accurate, the proposed change does not make them better.

[skirpichev]

But for Decimal methods this is incorrect.

No. But you are right, it's not documented how handled integer arguments for Decimal's methods. Perhaps, we should add analog of "Each Context method accepts a Python integer (an instance of int) anywhere that a Decimal instance is accepted." to the Decimal class docs.

Even if this is not incorrect for Context methods, it is verbose and repetitive. The current docstrings are accurate, the proposed change does not make them better.

I doubt they are accurate. For example, _pydecimal.Decimal.logical_and just says "Applies an 'and' operation between self and other's digits", which is wrong.

Maybe we should just sync docstrings of pure-Python and C-codec versions, ensure they have same set of examples and each mention, that operands are "logical" arguments (without - yes - a repetitive definitions)?