Conversation
This reverts commit 8d86bf3.
|
There's some kind of weird edge case we're hitting now sometimes with |
DRMacIver
left a comment
DRMacIver
left a comment
There was a problem hiding this comment.
Thanks for this. I've left some comments on things I think need improving or where I wasn't sure about the change, but I appreciate the work!
| # Attempt to replace | ||
| bin_search_down(0, self.result[i], lambda v: replace({i: v})) | ||
| i -= 1 | ||
| self.result = shrink_lower(self.result, consider) |
There was a problem hiding this comment.
Assigning result here is dodgy. It's not necessary (because calling the interestingness test automatically updates result) and can only introduce bugs.
There was a problem hiding this comment.
I hadn't considered that nearly enough. It makes factoring out the shrinking passes in the first place rather hairy: we're relying on a side-effect in the InterestingnessTest that gets passed in to actually update the result. Which is not to say it's bad, but it's not what I had in mind when I factored out the passes. I'll need to think about it a little bit.
There was a problem hiding this comment.
There are ways to shrink without side effects, see elm-microthesis. But it would need a larger refactor of the Python code than you perhaps intended.
| self.result = shrink_redistribute(self.result, consider) | ||
|
|
||
|
|
||
| def shrink_remove(current: array[int], is_interesting: InterestTest) -> array[int]: |
There was a problem hiding this comment.
Why are these external functions instead of methods on the runner?
|
|
||
|
|
||
| def shrink_remove(current: array[int], is_interesting: InterestTest) -> array[int]: | ||
| # Try removing chunks, starting from the end. |
There was a problem hiding this comment.
It would be nice if some of these comments were extracted into docstrings if these are to be extracted into their own functions.
| S = TypeVar("S", covariant=True) | ||
| U = TypeVar("U") # Invariant | ||
|
|
||
| InterestTest = Callable[[array], bool] # Really array[int] -> bool |
There was a problem hiding this comment.
I'd prefer InterestingnessTest
Also what's with the comment here? Why can't this be an array[int]?
There was a problem hiding this comment.
The builtin array type is not subscriptable, and so the language can't handle array[int] here
There was a problem hiding this comment.
oh, weird. I wonder if this is a Python version thing? https://github.com/python/typeshed/blob/main/stdlib/array.pyi#L21 makes it look like array should be able to handle subscripting.
There was a problem hiding this comment.
Confirmed works when running on 3.12
There was a problem hiding this comment.
3.11:
minithesis.py:85: in <module>
InterestTest = Callable[[array[int]], bool] # Really array[int] -> bool
E TypeError: type 'array.array' is not subscriptable3 participants
This PR mainly extracts shrinking passes into their own functions, and tries to write these functions to be a bit more legible.