Warn when an @st.composite strategy doesn't use the draw() function

[Zac-HD]

I've seen a few cases where users have missed the point of @st.composite, and ended up using it as a substitute for st.just():

@st.composite
def foo(_):
    return "bar"

I propose to fix this by using inspect.getsource()+ast.parse()+a custom AST visitor, which will raise an explanatory error (initially a deprecation warning) if the draw argument is not referenced in the function body. Note that the name of said argument is determined by the AST of the function signature, and the ctx of the node must be Load().

[rodrigogiraoserrao]

Me, @gzaxel, and @bomtall are going to be tackling this. Wish us luck 😜

[Jookia]

Is there a way to turn this off? I have code that wraps test code that may draw data but may not.

[bomtall]

It doesn’t need to call draw, only reference it Get Outlook for iOS<https://aka.ms/o0ukef>

…

________________________________ From: Jookia ***@***.***> Sent: Monday, July 18, 2022 11:13:30 AM To: HypothesisWorks/hypothesis ***@***.***> Cc: bomtall ***@***.***>; Mention ***@***.***> Subject: Re: [HypothesisWorks/hypothesis] Warn when an ***@***.***` strategy doesn't use the `draw()` function (Issue #3384) Is there a way to turn this off? I have code that wraps test code that may draw data but may not. — Reply to this email directly, view it on GitHub<#3384 (comment)>, or unsubscribe<https://github.com/notifications/unsubscribe-auth/AQ7AHQXCLD26JUZJALKLQP3VUUU4VANCNFSM5Z3TJJXA>. You are receiving this because you were mentioned.Message ID: ***@***.***>

[Jookia]

On Mon, Jul 18, 2022 at 03:23:40AM -0700, bomtall wrote: It doesn’t need to call draw, only reference it

Not entirely sure what that means. Anyway I've just solved this issue by pinning to 6.50.1. Thanks for the help.

[Zac-HD]

@Jookia can you post the function in question? This new check is quite precise, so if you have a warning it's probably a real problem.

[Jookia]

I have a decorate that runs a function with @composite wrapping it and giving it a 'draw' it might or might not need.

…

On Mon, Jul 18, 2022 at 07:56:45AM -0700, Zac Hatfield-Dodds wrote: @Jookia can you post the function in question? This new check is quite precise, so if you have a warning it's probably a real problem. -- Reply to this email directly or view it on GitHub: #3384 (comment) You are receiving this because you were mentioned. Message ID: ***@***.***>

[Zac-HD]

Again, your description is too vague for me to follow, if you can't post the actual source code I'm just going to move on.

[Jookia]

It's no big deal, I solved it with a pin.

[rodrigogiraoserrao]

I am going to go out on a limb here and say that pinning Hypothesis isn't really _solving_ your problem. The check that was added has the purpose of revealing logical issues with your code. If something is decorated with `st.composite`, it was so because **it needs** to draw things. If it doesn't need to draw stuff, it doesn't need to be tagged with `st.composite`. So, in conclusion, if your code is triggering the deprecation warning, it's because it is _highly likely_ that there is a better (that is, simpler, more elegant) way of doing whatever it is that you really want to be doing. Of course, if you don't add any details, Zac won't be able to help you 🤷

[Jookia]

Look, this change has rubbed me the wrong way: It doesn't deprecate a feature or code, instead goes out of the way to break code by walking the AST and effectively doing linting that I didn't ask for. Nowhere in the documentation has it even been suggested that you need to draw things in all code that uses st.composite and I'm effectively being punished for writing code 'wrong' despite following all the rules and best practices I've had access to. I have no idea what's going to be restricted next, so pinning solves this for my immediate future before more of my code is crippled.