Skip to content

DecimalTuple can have string exponent #9194

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.

Sign up for GitHub

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

Jump to bottom
Merged
srittau merged 1 commit into from
Nov 14, 2022
Merged

DecimalTuple can have string exponent #9194

srittau merged 1 commit into from
Nov 14, 2022

Conversation

@sobolevn
Copy link
Member

@sobolevn sobolevn commented Nov 14, 2022

@github-actions
Copy link
Contributor

github-actions bot commented Nov 14, 2022

Diff from mypy_primer, showing the effect of this PR on open source code:

mongo-python-driver (https://github.com/mongodb/mongo-python-driver)
+ bson/decimal128.py: note: In function "_decimal_to_128":
+ bson/decimal128.py:103: error: No overload variant of "__add__" of "str" matches argument type "int"  [operator]
+ bson/decimal128.py:103: note: Possible overload variants:
+ bson/decimal128.py:103: note:     def __add__(self, str, /) -> str
+ bson/decimal128.py:103: note: Left operand is of type "Union[int, Literal['n', 'N', 'F']]"

spark (https://github.com/apache/spark)
+ python/pyspark/sql/connect/column.py:114: error: Argument 1 to "abs" has incompatible type "Union[int, Literal['n', 'N', 'F']]"; expected "SupportsAbs[int]"  [arg-type]
+ python/pyspark/sql/connect/column.py:115: error: Argument 1 to "abs" has incompatible type "Union[int, Literal['n', 'N', 'F']]"; expected "SupportsAbs[int]"  [arg-type]

pydantic (https://github.com/samuelcolvin/pydantic)
+ pydantic/_internal/_validators.py:160: error: Unsupported operand types for >= ("Literal['n']" and "int")  [operator]
+ pydantic/_internal/_validators.py:160: error: Unsupported operand types for >= ("Literal['N']" and "int")  [operator]
+ pydantic/_internal/_validators.py:160: error: Unsupported operand types for >= ("Literal['F']" and "int")  [operator]
+ pydantic/_internal/_validators.py:160: note: Left operand is of type "Union[int, Literal['n', 'N', 'F']]"
+ pydantic/_internal/_validators.py:162: error: Unsupported operand types for + ("int" and "Literal['n']")  [operator]
+ pydantic/_internal/_validators.py:162: error: Unsupported operand types for + ("int" and "Literal['N']")  [operator]
+ pydantic/_internal/_validators.py:162: error: Unsupported operand types for + ("int" and "Literal['F']")  [operator]
+ pydantic/_internal/_validators.py:162: note: Right operand is of type "Union[int, Literal['n', 'N', 'F']]"
+ pydantic/_internal/_validators.py:170: error: Argument 1 to "abs" has incompatible type "Union[int, Literal['n', 'N', 'F']]"; expected "SupportsAbs[int]"  [arg-type]
+ pydantic/_internal/_validators.py:171: error: Argument 1 to "abs" has incompatible type "Union[int, Literal['n', 'N', 'F']]"; expected "SupportsAbs[int]"  [arg-type]
+ pydantic/_internal/_validators.py:174: error: Argument 1 to "abs" has incompatible type "Union[int, Literal['n', 'N', 'F']]"; expected "SupportsAbs[int]"  [arg-type]
+ pydantic/json.py:38: error: Unsupported operand types for >= ("Literal['n']" and "int")  [operator]
+ pydantic/json.py:38: error: Unsupported operand types for >= ("Literal['N']" and "int")  [operator]
+ pydantic/json.py:38: error: Unsupported operand types for >= ("Literal['F']" and "int")  [operator]
+ pydantic/json.py:38: note: Left operand is of type "Union[int, Literal['n', 'N', 'F']]"

psycopg (https://github.com/psycopg/psycopg)
+ psycopg/psycopg/types/numeric.py:394: error: Unused "type: ignore" comment
+ psycopg/psycopg/types/numeric.py:396: error: Unused "type: ignore" comment

@sobolevn
Copy link
Member Author

sobolevn commented Nov 14, 2022

This failure is not related: https://github.com/python/typeshed/actions/runs/3460013243/jobs/5776061994 I cannot restart it.

Looking at the primer results ⌛

@sobolevn
Copy link
Member Author

sobolevn commented Nov 14, 2022

mongo's code is correct. But, we cannot narrow exponent based on other calls: https://github.com/mongodb/mongo-python-driver/blob/fcb11514506acddbd50b8ec13e76a7c34d336aac/bson/decimal128.py#L81-L83

    sign, digits, exponent = value.as_tuple()

    if value.is_nan():

pyspark's code is broken: https://github.com/apache/spark/blob/632784dec8a0c682a85a888c66e83f709e402e18/python/pyspark/sql/connect/column.py#L114-L115

It is calling abs() on a possibly string. They even state that raise ValueError("Python Decimal not supported.")

pydantic has correct runtime code: https://github.com/pydantic/pydantic/blob/13ad034e4400baf5a875767e363aba879501065f/pydantic/_internal/_validators.py

            _, digit_tuple, exponent = value.as_tuple()
            if not self.allow_inf_nan and exponent in {'F', 'n', 'N'}:
                raise PydanticKnownError('finite_number')

            if self.check_digits:
                if exponent >= 0:

But, looks like we cannot narrow exponent to be int in this case :(

Code in psycopg is actually correct (it has type: ignore that will be removed): https://github.com/psycopg/psycopg/blob/a8168869b762c821e7f07d1fc52d3642f899711e/psycopg/psycopg/types/numeric.py#L393-L397

@srittau srittau merged commit e8111ea into python:main Nov 14, 2022
@hauntsaninja hauntsaninja mentioned this pull request Nov 18, 2022
Merged
@ShaneHarvey ShaneHarvey mentioned this pull request Apr 22, 2023
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@srittau srittau srittau approved these changes

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

DecimalTuple definition is incorrect

2 participants

@sobolevn @srittau