faker icon indicating copy to clipboard operation
faker copied to clipboard

pydecimal() - `min_value` and `max_value` should support `Decimal`, along with `float`

Open sshishov opened this issue 1 year ago • 10 comments
trafficstars

  • Faker version: 25.0.1
  • OS: MacOS 14.4.1 (23E224)

After migrating to version 25+, profiding default value as Decimal for min or max values causes violation

Steps to reproduce

amount=fake.pydecimal(min_value=Decimal(1), max_value=Decimal(100000)),

Expected behavior

The should not be any error observed. The rationale - a lot of software using Decimal from the box for all calculations, in the constants etc to rely on precise double calculation. Therefore I think that it should support Decimal from the box, especially if it is returning Decimal itself.

Actual behavior

Produces this mypy error:

 error: Argument "min_value" to "pydecimal" of "Faker" has incompatible type "Decimal"; expected "float | None"  [arg-type]

sshishov avatar May 05 '24 17:05 sshishov

This is a typing issue, no "real" bug. Feel free to provide a PR to update the corresponding type hint and regenerate the typing stubs.

stefan6419846 avatar May 05 '24 17:05 stefan6419846

@stefan6419846 how I can report typing issues? Because we are havily using faker, and we are requiring strict typing, that's why I have noticed these errors...

If you will give me a way to report typing issues, then I will report there.

sshishov avatar May 05 '24 18:05 sshishov

It is indeed correct to report this here, but your initial report sounded like an implementation bug and not a type hint issue.

stefan6419846 avatar May 05 '24 18:05 stefan6419846

If we want to fix that, we can add more annotation in code

    def pydecimal(
        self,
        left_digits: Optional[int] = None,
        right_digits: Optional[int] = None,
        positive: bool = False,
        min_value: Optional[Union[float, int, Decimal]] = None,
        max_value: Optional[Union[float, int, Decimal]] = None,
    ) -> Decimal:

but I think a problem is, we can have another types of numbers that are subclass of int or something like this, how we can handle that in here?

parsariyahi avatar May 13 '24 08:05 parsariyahi

Hi @parsariyahi , any subclass should be okay, I guess. We need to check though...

sshishov avatar May 28 '24 12:05 sshishov

After some research, I found something, the built-in numbers module, this module has a class Number which is a abstract base class for all numbers I think. you can read it here: https://docs.python.org/3/library/numbers.html as the docs said:

The numbers module (PEP 3141) defines a hierarchy of numeric abstract base classes which progressively define more operations. None of the types defined in this module are intended to be instantiated.

I think it's fit the needs for this annotation

parsariyahi avatar May 28 '24 17:05 parsariyahi

@parsariyahi using numbers.Number seems like the right solution here. Do y ou have time to prepare a Pull Request?

fcurella avatar Jun 03 '24 14:06 fcurella

@fcurella Yes sure, you can asign this issue to me, I will prepare a PR

parsariyahi avatar Jun 03 '24 14:06 parsariyahi

turns out, Decimal is not part of numbers.Number.

I've tried using Union[Decimal, int, float], but looks like abs() doesn't accept Decimal, so the best I can do is to define a new type BasicNumber = [int, float]

fcurella avatar Jun 20 '24 14:06 fcurella

@fcurella Could you share a bit more about what you wrote above?

but looks like abs() doesn't accept Decimal,

abs() says:

The argument may be an integer, a floating-point number, or an object implementing __abs__().

And we have:

In [12]: Decimal.__abs__?
Signature:      Decimal.__abs__(self, /)
Call signature: Decimal.__abs__(*args, **kwargs)
Type:           wrapper_descriptor
String form:    <slot wrapper '__abs__' of 'decimal.Decimal' objects>
Docstring:      abs(self)

Plus it's clearly possible to call abs(Decimal(-1)) and get a reasonable result.

So I'm confused why you'd say it's not supported - could you help me understand 🙏🏻

jamescooke avatar Sep 17 '24 20:09 jamescooke

@fcurella the decimal.Decimal is the part of number.Number as far as I can tell.

You can even test it like this (Python 3.11.10):


In [1]: import numbers

In [2]: import decimal

In [3]: issubclass(decimal.Decimal, numbers.Number)
Out[3]: True

In [4]: isinstance(decimal.Decimal(100), numbers.Number)
Out[4]: True

I have tried the new version and all the complains disappeared for me. Am I missing something?

sshishov avatar Nov 19 '24 21:11 sshishov

The PEP for Decimal states:

After consultation with its authors it has been decided that the Decimal type should not at this time be made part of the numeric tower.

I've tried replacing the type with numbers.Number, but mypy v1.13.0 on Python 3.10 errors out:

$ mypy --install-types --non-interactive --config mypy.ini faker
faker/providers/python/__init__.py:299:64: error: Unsupported left operand type for > ("Number")  [operator]
            if min_value is not None and max_value is not None and min_value > max_value:
                                                                   ^~~~~~~~~~~~~~~~~~~~~
faker/providers/python/__init__.py:303:51: error: Unsupported operand types for >= ("int" and "Number")  [operator]
            if positive and min_value is not None and min_value <= 0:
                                                      ^
faker/providers/python/__init__.py:305:79: error: Argument 1 to "abs" has incompatible type "Number"; expected "SupportsAbs[Never]"  [arg-type]
            if left_digits is not None and max_value and math.ceil(math.log10(abs(max_value))) > left_digits:
                                                                                  ^~~~~~~~~
faker/providers/python/__init__.py:307:79: error: Argument 1 to "abs" has incompatible type "Number"; expected "SupportsAbs[Never]"  [arg-type]
            if left_digits is not None and min_value and math.ceil(math.log10(abs(min_value))) > left_digits:
                                                                                  ^~~~~~~~~
faker/providers/python/__init__.py:314:38: error: Argument 1 to "abs" has incompatible type "Number | int"; expected "SupportsAbs[int]"  [arg-type]
                math.ceil(math.log10(abs(min_value or 1))),
                                         ^~~~~~~~~~~~~~
faker/providers/python/__init__.py:315:38: error: Argument 1 to "abs" has incompatible type "Number | int"; expected "SupportsAbs[int]"  [arg-type]
                math.ceil(math.log10(abs(max_value or 1))),
                                         ^~~~~~~~~~~~~~
faker/providers/python/__init__.py:319:38: error: Unsupported operand types for <= ("int" and "Number")  [operator]
            if min_value is not None and min_value >= 0:
                                         ^
faker/providers/python/__init__.py:321:40: error: Unsupported operand types for >= ("int" and "Number")  [operator]
            elif max_value is not None and max_value <= 0:
                                           ^
faker/providers/python/__init__.py:331:55: error: Value of type variable "SupportsRichComparisonT" of "max" cannot be "Number | int"  [type-var]
                    left_number = str(self.random_int(int(max(min_value or 0, 0)), int(max_value)))
                                                          ^~~~~~~~~~~~~~~~~~~~~~
faker/providers/python/__init__.py:331:55: error: Argument 1 to "int" has incompatible type "Number | int"; expected "str | Buffer | SupportsInt | SupportsIndex | SupportsTrunc"  [arg-type]
                    left_number = str(self.random_int(int(max(min_value or 0, 0)), int(max_value)))
                                                          ^~~~~~~~~~~~~~~~~~~~~~
faker/providers/python/__init__.py:331:80: error: No overload variant of "int" matches argument type "Number"  [call-overload]
                    left_number = str(self.random_int(int(max(min_value or 0, 0)), int(max_value)))
                                                                                   ^~~~~~~~~~~~~~
faker/providers/python/__init__.py:331:80: note: Possible overload variants:
faker/providers/python/__init__.py:331:80: note:     def __new__(cls, str | Buffer | SupportsInt | SupportsIndex | SupportsTrunc = ..., /) -> int
faker/providers/python/__init__.py:331:80: note:     def __new__(cls, str | bytes | bytearray, /, base: SupportsIndex) -> int
faker/providers/python/__init__.py:333:56: error: Value of type variable "SupportsRichComparisonT" of "max" cannot be "Number | int"  [type-var]
                    min_left_digits = math.ceil(math.log10(max(min_value or 1, 1)))
                                                           ^~~~~~~~~~~~~~~~~~~~~~
faker/providers/python/__init__.py:333:56: error: Argument 1 to "log10" has incompatible type "Number | int"; expected "SupportsFloat | SupportsIndex"  [arg-type]
                    min_left_digits = math.ceil(math.log10(max(min_value or 1, 1)))
                                                           ^~~~~~~~~~~~~~~~~~~~~~
faker/providers/python/__init__.py:339:59: error: Value of type variable "SupportsRichComparisonT" of "min" cannot be "Number | int"  [type-var]
                    left_number = str(self.random_int(int(abs(min(max_value or 0, 0))), int(abs(min_value))))
                                                              ^~~~~~~~~~~~~~~~~~~~~~
faker/providers/python/__init__.py:339:59: error: Argument 1 to "abs" has incompatible type "Number | int"; expected "SupportsAbs[int]"  [arg-type]
                    left_number = str(self.random_int(int(abs(min(max_value or 0, 0))), int(abs(min_value))))
                                                              ^~~~~~~~~~~~~~~~~~~~~~
faker/providers/python/__init__.py:339:93: error: Argument 1 to "abs" has incompatible type "Number"; expected "SupportsAbs[Never]"  [arg-type]
                    left_number = str(self.random_int(int(abs(min(max_value or 0, 0))), int(abs(min_value))))
                                                                                                ^~~~~~~~~
faker/providers/python/__init__.py:341:60: error: Value of type variable "SupportsRichComparisonT" of "min" cannot be "Number | int"  [type-var]
                    min_left_digits = math.ceil(math.log10(abs(min(max_value or 1, 1))))
                                                               ^~~~~~~~~~~~~~~~~~~~~~
faker/providers/python/__init__.py:341:60: error: Argument 1 to "abs" has incompatible type "Number | int"; expected "SupportsAbs[int]"  [arg-type]
                    min_left_digits = math.ceil(math.log10(abs(min(max_value or 1, 1))))
                                                               ^~~~~~~~~~~~~~~~~~~~~~
faker/providers/python/__init__.py:355:47: error: Unsupported operand types for > ("Decimal" and "Number")  [operator]
            if max_value is not None and result > max_value:
                                                  ^~~~~~~~~
faker/providers/python/__init__.py:357:47: error: Unsupported operand types for < ("Decimal" and "Number")  [operator]
            if min_value is not None and result < min_value:
                                                  ^~~~~~~~~
Found 20 errors in 1 file (checked 672 source files)

fcurella avatar Nov 20 '24 21:11 fcurella

This issue is stale because it has been open for 30 days with no activity.

github-actions[bot] avatar Feb 19 '25 01:02 github-actions[bot]

This issue was closed because it has been inactive for 14 days since being marked as stale.

github-actions[bot] avatar Mar 05 '25 02:03 github-actions[bot]