Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

SKIP doesn't behave as expected #107

Open
mathrick opened this issue Oct 18, 2019 · 7 comments
Open

SKIP doesn't behave as expected #107

mathrick opened this issue Oct 18, 2019 · 7 comments

Comments

@mathrick
Copy link

Trivial example:

>>> glom({'foo': 42}, ('foo', lambda x: SKIP))
42

SKIP, in a tuple, essentially results in whatever spec produced it not being there, rather than the entire compound spec (ie. the surrounding tuple) coming to a halt. That is not super important in the example above, but I constantly run into it in my glom-based parser / pattern matcher, where I have a potential match that needs further processing:

glom(data, (lambda x: x if x.get('class') == 'foo' else SKIP, 'val', str))

I'd expect SKIP to essentially abort the entire "branch" of a computation (which is what a tuple of spec is, really), but instead it happily lets through whatever was the target before, leading to a lot unexpected results.

@mahmoud
Copy link
Owner

mahmoud commented Oct 18, 2019

Hi Maciej! I've struggled a bit with SKIP in tuples myself, in fact. SKIP's meant to be used for optional processing steps in the tuple. For aborting the processing, you'd want to use STOP. Give that a whirl and let me know what you think. :)

@mathrick
Copy link
Author

@mahmoud: STOP was in fact the first thing I tried when I ran into it :). But it doesn't really abort either, just prevents further steps from taking place. For example:

glom({'data': {'class': 'foo', 'val': 42}}, 
     ('data', lambda x: x if x.get('class') == 'bar' else STOP))
>>> {'class': 'foo', 'val': 42}

@mahmoud
Copy link
Owner

mahmoud commented Oct 18, 2019

Ah, so the desired behavior is that the target would get dropped if a SKIP (or maybe a STOP) is returned from the tuple? Actually, if you wouldn't mind including the desired output for the example(s) above that'd be great!

@mahmoud
Copy link
Owner

mahmoud commented Oct 23, 2019

@mathrick just a gentle ping on the example; I'm still thinkin about this one!

@mathrick
Copy link
Author

mathrick commented Oct 24, 2019

Ah, sorry, I got busy with another part of my project and I forgot to reply here. The output is a bit of a tricky area, since in the example above, you really want "nothing" (ie. no values at all, not even None), which is not possible. I think returning a SKIP here would be acceptable, since it's a sentinel value already, and the client code will just need to check for that. The advantage of doing so would be that it'd compose nicely inside specs, so SKIP and STOP always mean "abort my chain of computation" (the difference being that STOP also aborts any sibling chains). Determining the scope of the abort would then be the job of individual specs. For example, in a fictional webapp which needs to display a user's name:

display_name = glom(user,
                    (Check(has_completed_profile, default=STOP),
                     Coalesce(
                         (Check(has_new_profile, default=SKIP), 'profile.display_name'),
                         (Check(has_legacy_profile, default=SKIP), 'user_profile.name')),
                     format_for_display))

if display_name == SKIP:
    raise redirect(url_for(complete_user_profile))

Here Check serves as a guard condition, ensuring that the rest of the access can only happen if the check passes. The first Check will not let the Coalesce execute at all if it fails, thanks to the STOP. While the above is a made-up example, it's actually pretty close in spirit to what my use case is, whilst being a bit easier to understand without needing an introduction to my homegrown pattern matching syntax :)

If SKIP as the return value doesn't sit right with you, then perhaps a new sentinel NOTHING would be better?

@mathrick
Copy link
Author

@mahmoud: just a quick ping, is this clear enough, or do you need more info?

@mahmoud
Copy link
Owner

mahmoud commented Oct 31, 2019

Hmm, yeah I think more info would be helpful. The code blocks above detail some of the data and some prospective specs, so really the only thing remaining would be the expected output. Would love to see that.

By the by, we're working on a Match spec, somewhat positioned as a successor to Check in certain cases, over in #94. Always good to take more use cases into account there.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants