-
Notifications
You must be signed in to change notification settings - Fork 586
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
FailedHealthCheck for decimals(places=X, allow_nan=False, allow_infinity=False) #725
Comments
|
@DRMacIver, I'm pretty sure that #710 broke this. Any ideas? |
|
@Zac-HD it is indeed #710 that broke it, but I think it's a bug that's been latent since #508 that is now being triggered consistently by the new larger integers (which are particularly egregious in health checks because it runs in pure random mode). The specific problem is that we're now hitting the InvalidOperation branch here 100% of the time in health checks (why do we have a no cover there anyway?). I think the problem is that we're using an unrestricted Do you want to look into doing that or should I? (I'm totally happy to but don't have a lot of bandwidth to do so for the next few days) |
|
I'm certainly happy to take a look, though probably also not in the next few days - I really want to finish #643 before my talk at the August meetup and the PyConAU sprints so that's got priority. |
|
@goodspark it looks like we probably won't to be able to get to this super soon, sorry. If you want to take a look at the code linked above and try to put together a patch, we can help answer questions etc. No worries if not though. In the meantime as a workaround, my guess (which I've not validated) is that this will work better than the health checks are advertising. If you add |
|
@Zac-HD Totally reasonable! I'll let you know if I start working on it before you do. |
|
FWIW when I recreate the (equivalent) code locally and generate examples, there are only a few cases where from hypothesis.strategies import integers
from hypothesis.strategies import sampled_from
from decimal import InvalidOperation
from decimal import Decimal
def try_quantize(d):
try:
return d.quantize(factor)
except InvalidOperation:
return None
factor = Decimal(10) ** -3
# same code but without last filter to see how many quantize failures are happening
strat = integers().map(lambda d: try_quantize(d * factor)) | sampled_from([])
vals = [strat.example() for _ in range(10000)]
nones = [x for x in vals if x is None]
float(len(nones))/len(vals) * 100
> 3.09 |
|
Yeah, there's a difference in how the health checks run from how things run in the normal engine. Unifying the two is something I'll probably be looking into in the near future if things go according to plan. |
|
Ah, got it - this happens when the underlying decimal (draw integer, multiply by Decimal(10)^-places) has greater precision than allowed in the decimal context. The engine changes in 3.12 made large integers (~ 10^34 ) much more common, and thus we have issues. This is easily fixed by making the default bounds +- 10^prec when a max or min value is not given. I'll also add validation for user-supplied enormous inputs, and make a pull request after investigating #739. |
My tests have suddenly started failing with v3.12.0 due to failing health checks. Worked in v.3.11.6
Minimum test code:
If I remove
places, this works again.Output:
The text was updated successfully, but these errors were encountered: