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

fix: pyfloat type error on none max value #1908

Merged
merged 1 commit into from
Sep 19, 2023

Conversation

HugoJP1
Copy link
Contributor

@HugoJP1 HugoJP1 commented Sep 8, 2023

What does this change

Handle the combinations of specified digits and min and max values in a more advanced way for pyfloat.

What was wrong

There were a few issues that this PR aims to address:

  • This line was throwing a TypeError if max_value or min_value was None
self.generator.random.uniform(0, max_value - min_value)
  • This line would throw an error when needed_left_digits == sys.float_info.dig because the range was then invalid
  • Giving in -(10**sys.float_info.dig + 1) as max value and 10**sys.float_info.dig as min_value can never succeed because of the following code at the end:
        if right_digits:
            result = min(result, 10**left_digits - float(f'0.{"0" * (right_digits - 1)}1'))
            result = max(result, -(10**left_digits + float(f'0.{"0" * (right_digits - 1)}1')))
        else:
            result = min(result, 10**left_digits - 1)
            result = max(result, -(10**left_digits + 1))
  • The number of left and right digits were not always respected.

How this fixes it

  • ValueError is raised if max_value or min_value is out of bounds
  • right digits is set to 0 if the inferred left digits is already equals to sys.float_info.dig
  • Rather than choosing only the left_number which then caused issues in trying to figure out a "right number" which could then end up being lower/higher than the configured max/min value, an int is chosen randomly between min and max values which are scaled up to have the length of left + right digits so that afterwards a . can be inserted in the right place.
  • The previous adjustment code containing the TypeError is removed.

Fixes #1907

Comment on lines +199 to +208
if max_value is None:
temp_max_value = 10 ** (left_digits + right_digits)
if min_value is not None:
temp_min_value = min_value * 10 ** right_digits
if min_value is None:
temp_min_value = -(10 ** (left_digits + right_digits))
if max_value is not None:
if max_value < temp_min_value:
temp_min_value = max_value - 1
temp_max_value = max_value * 10 ** right_digits
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This section aims to infer boundaries in the case where we don't have max_value or min_value.

Comment on lines +196 to +197
temp_min_value = min_value
temp_max_value = max_value
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Renamed to temp rather than left because it is now doing both the right and left parts.

Comment on lines +215 to +227
opposite_signs = (temp_min_value * temp_max_value) < 0
if opposite_signs and left_digits_requested:
positive_number = self._safe_random_int(
10 ** (left_digits + right_digits - 1),
temp_max_value,
positive=True,
)
negative_number = self._safe_random_int(
temp_min_value,
-(10 ** (left_digits + right_digits - 1)),
positive=False,
)
number = self.random_element((positive_number, negative_number))
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This part is so that we can generate a number which respects these two constraints:

  1. It must have a fixed amount of left digits
  2. It can be either positive or negative.

In this case we need to choose two numbers, one in the negative side, and one in the positive. We then choose randomly between the two.

@HugoJP1 HugoJP1 changed the title fix: type error on none max value fix: pyfloat type error on none max value Sep 8, 2023
Copy link
Collaborator

@fcurella fcurella left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you! <3

@fcurella fcurella merged commit d2ce122 into joke2k:master Sep 19, 2023
24 checks passed
fcurella added a commit that referenced this pull request Oct 11, 2023
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

Successfully merging this pull request may close these issues.

Pyfloat raises TypeError for None min or max value
2 participants