Skip to content
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

pydecimal does not properly handle positive keyword #2051

Open
sshishov opened this issue May 28, 2024 · 1 comment
Open

pydecimal does not properly handle positive keyword #2051

sshishov opened this issue May 28, 2024 · 1 comment

Comments

@sshishov
Copy link
Contributor

The same issue is happening for pydecimal as it was reported for pyfloat: #1928

I think that it should work more straightforward and more reliable, like: we should have a min and max values being updated and only at the end we generate the value.

MIN:

  • min unset, positive unset, result = minimum calculated from places
  • min set, positive unset, result = maximum of min and minimum calculated from places
  • min unset, positive set to True, result = maximum between 0 and minimum calculated from places
  • min unset, positive set to False, result = minimum calculated from places
  • min set and positive set to True, result = maximum between 0 and min
  • min set and positive set to False, result = maximum of min and minimum calculated from places

MAX:

  • max unset, positive unset, result = maximum calculated from places
  • max set, positive unset, result = minimum of max and maximum calculated from places
  • max unset, positive set to True, result = maximum calculated from places
  • max unset, positive set to False, result = minimum between 0 and maximum calculated from places
  • max set and positive set to True, result = minimum of max and maximum calculated from places
  • max set and positive set to False, result = minimum between 0 and max

Afterwards we should check min <= max and then return the random value from this range

I guess this approach is safe and reliable and will indicate about issues in usage when you try to build pydecimal from constrains which produce invalid range

@parsariyahi
Copy link
Contributor

I was reading the source code and I saw that the same code in #1928 is used here

        if min_value is not None and min_value >= 0:
            sign = "+"
        elif max_value is not None and max_value <= 0:
            sign = "-"
        else:
            sign = "+" if positive else self.random_element(("+", "-"))

So, if we get to else statement the sign will choose randomly, and I think that's a bug.
But I was thinking about it and I didn't find any usage for a positive param in pydecimal fucntion.

So let's say we want negative numbers, we set max to 0 and the code should be functioning.
And other hand let's sat I don't want any negatives, so min to 0 and now I know I won't get any.
Or I don't care about the sign, and don't use any of min or max and the code should be random for +, -.

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

No branches or pull requests

2 participants