search

Seelengrab / Supposition.jl

A Julia implementation of choice sequence based PBT, inspired by Hypothesis
https://seelengrab.github.io/Supposition.jl/
European Union Public License 1.2
46 stars 2 forks source link

Rename `assume!` to `assume` #12

Closed jariji closed 8 months ago

jariji commented 8 months ago

https://docs.julialang.org/en/v1/manual/style-guide/#bang-convention gives the convention

Append ! to names of functions that modify their arguments

Since assume!(::Bool) doesn't modify its arguments I suspect it should be assume(::Bool).

Seelengrab commented 8 months ago

Unlike rand (where the hidden state is always assumed to exist), the currently used test case does not always exist, so the bang serves as a reminder that there is some (hidden) state being modified when this is called. I'm feeling a bit uneasy about removing the bang there (and similarly with target! and reject! too) because of that.

It's a bit like Random.seed!(), which seeds a (hidden) default.

jariji commented 8 months ago

I like having a simple rule with a specific universal documented meaning so authors can use it to communicate properties of a function simply and precisely to their users. Unfortunately the current "convention" doesn't follow the rule quoted above; it's subjective, as seen in idiosyncrasies like rand(rng), print(::IOBuffer), and seed!(k), which makes it hard to use for precise communication between individuals.

Anyway that's just my personal rant, not your problem. Feel free to close this if you like.

Seelengrab commented 8 months ago

I'll do a small export-audit of everything that I consider API before registering, so I'll take a look at this then.

Seelengrab commented 8 months ago

Alright, I've reworked the exposed API. Everything now consistently uses <verb>! from the user-side, to indicate the change of hidden state. I've also modified the internal uses to follow the usual convention.