Repository navigation
Conversation
…alues meet_into and meet_k_path_into with prune = true now remove every dangling path below the focus, not only the chain at the focus, then prune the focus as before. With prune = false a meet may leave dangling paths (which ones is unspecified), but the status and return value depend only on the values: a result holding none is None / false. meet_2, which has no prune flag, follows prune = false. Two tests encoded the old rule and are updated: write_zipper_meet_into_test4 kept a dangling-only intersection under prune = true and reported Element, and write_zipper_prune_flag_test assumed prune = true equals prune = false plus prune_path at the focus, which no longer holds for meet_into.
|
This change in behavior is (at least partially) counter-productive, IMO. The rationale behind meet not giving first-class treatment to dangling paths is performance. But instituting a reliable policy of "no dangling path survives a meet" (which this PR does) is just as costly to maintain as first-class treatment of dangling paths in meet. (Actually more costly within the meet call, it's just that it hopefully pays back in future traversals avoided.) Additionally, this PR causes meet_into's prune flag to behave differently from the flag on every other op. Every other op's prune flag is documented to be identical to calling |
|
This PR is as good a place to have the discussion as any, since the other meet behavior PR is closed. First, let me introduce a theoretical API concept: If trimming a subtrie is desirable, merging a But a thorough Which gets to a third thing that I'll call So there are 3 possible contracts for a meet-like operation.
Each one has a situation where it's the clear winner. I hate that the contract for I guess if I had my preference, it would be to implement meet the "clean" way with full guarantees around first class paths, and provide a |
What does it mean to have first class paths in meet? What guaranteed do you want to provide?
I'd have expected this phrasing for e.g. zipper re-use, not for a huge asymptotic gap in the algebra. If we want to first-class dangling paths, we can, but then a lot is going to change with a lot of work; so there'd have to be a use case (e.g. someone asking about dangling paths under algebraic ops) |
It means meet upholds these laws:
I call it "dirty" because it makes vastly fewer guarantees about the behavior or the final result. The guarantees of Therefore there are 3 other cases are: We can choose a different word from For completeness, the guarantees of
What would need to change? It's practically what we already have minus what essentially can be thought of as "bugs". I.e. there is certainly nobody currently relying on dangling paths not being first class, because the current implementation is somewhere between the "first_class_dangling_paths" implementation and the |
|
Also worth saying - we could extend the other ops ( If we're working from the pragmatic frame that "having dangling paths is a cost, so they should be cleaned up opportunistically, but spending immediate resources to exhaustively trim them out is not a good tradeoff", having |
|
I don't like the idea of exploding the API surface with extra names. |
Regarding arg vs. additional entry point, I have a couple of opinions / observations. But it's mainly an aesthetic taste thing.
|
meet_into and meet_k_path_into with prune = true now remove every dangling path below the focus, not only the chain at the focus, then prune the focus as before. With prune = false a meet may leave dangling paths (which ones is unspecified), but the status and return value depend only on the values: a result holding none is None / false. meet_2, which has no prune flag, follows prune = false.
Two tests encoded the old rule and are updated: write_zipper_meet_into_test4 kept a dangling-only intersection under prune = true and reported Element, and write_zipper_prune_flag_test assumed prune = true equals prune = false plus prune_path at the focus, which no longer holds for meet_into.