glom icon indicating copy to clipboard operation
glom copied to clipboard

Switching SKIP and STOP to exception control flow

Open kurtbrose opened this issue 4 years ago • 2 comments

agree we shouldn't modify Fill to respect SKIP and STOP.... maybe Match though?

kurtbrose avatar Aug 04 '20 01:08 kurtbrose

Hmm, I think "tests are green" may be a bit of an overstatement. CI is red and many existing tests were changed.

I see what you're getting at though. It's certainly a purer approach. But it does significantly change/break the behavior of SKIP and STOP, both of which have been part of the API for a very long time. They're going from purely sentinel values, controlling the flow from inside the target, to purely specifier types controlling the flow from inside the spec.

Assuming we need to go this direction (and I can think of reasons, but would like to see them demonstrated), the right way to do this would be to have at least one version (but realistically some time window) where we respect the current behavior but warn.

mahmoud avatar Aug 04 '20 03:08 mahmoud

pytest is green locally which took a couple of hours of fiddling

We do need at a minimum a continue construct; I thought that could be reconciled with SKIP but now I'm not so sure. (I couldn't really tell without trying.)

The argument for why the exception flow is better from scratch is that it let's you avoid accidental further execution ; however this idea is messed up by tuple intercepting SKIP

So going down the route of adding a new construct -- SKIP can either be a sentinel value AND a spec; or there cold be new CONT and BREAK spec

I think one of the best arguments for exception based control flow is how a bunch of lambda and Val were removed from mtch and group tests; test_basic needs T arithmetic and then will be much cleaner as well

The best argument in favor of sentinel values is default=SKIP argument... I wonder if this cold be addressed by default being argmode rather than just a literal

As I write this... I think the thing to do is keep SKIP and STOP and add CONT and BREAK

There's a separate discussion to be had then about if SKIP and STOP should exist long term, but if the answer is no they should be retired gradually

On Mon, Aug 3, 2020, 8:50 PM Mahmoud Hashemi [email protected] wrote:

Hmm, I think "tests are green" may be a bit of an overstatement. CI is red and many existing tests were changed.

I see what you're getting at though. It's certainly a purer approach. But it does significantly change/break the behavior of SKIP and STOP, both of which have been part of the API for a very long time. They're going from purely sentinel values, controlling the flow from inside the target, to purely specifier types controlling the flow from inside the spec.

Assuming we need to go this direction (and I can think of reasons, but would like to see them demonstrated), the right way to do this would be to have at least one version (but realistically some time window) where we respect the current behavior but warn.

— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub https://github.com/mahmoud/glom/pull/183#issuecomment-668363969, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAEEZPS46X4ZPZYOX6N6LADR66APTANCNFSM4PT4GUVQ .

kurtbrose avatar Aug 04 '20 13:08 kurtbrose