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

Scope usage changes from v22 to v23 #259

Open
DamianBarabonkovQC opened this issue Apr 25, 2023 · 7 comments
Open

Scope usage changes from v22 to v23 #259

DamianBarabonkovQC opened this issue Apr 25, 2023 · 7 comments

Comments

@DamianBarabonkovQC
Copy link

In glom v22, I was able to assign a scope with a spec and that automatically got applied to the target. Version 23 breaks that.

For example, this code used to work in v22:

target = {'data': {'val': 9}}
spec = (S(value = ('data', 'val')), S.value)
glom(target, spec) # returns 9

However, in v23

target = {'data': {'val': 9}}
spec = (S(value = ('data', 'val')), S.value)
glom(target, spec) # returns ('data', 'val')

The previous behavior of v22 is infinitely more helpful than this new usage. Unless this was an intentional breaking change, I am filing this issue to recommend that we revert to the old behavior.

@mahmoud
Copy link
Owner

mahmoud commented Apr 26, 2023

Ah yeah this looks related to the arg-mode update (#196). You can get the old behavior with S(value=Path('data', 'val'))

Previous to arg mode, the default interpretation of S kwargs as literals vs specs was somewhat arbitrary/unspecified. I believe now that it's specified, we'd prefer to keep it this way, and because Path is a forward-compatible approach, hopefully that approach can work for this case. Your feedback on S usage is valuable, so give it a try and let us know :)

@kurtbrose
Copy link
Collaborator

Could you give more context on what you use that for?

You can get back the old behavior via explicit mode (Auto) or disambiguate by using T --

>>> import glom
>>> from glom import S, glom, Auto
>>> target = {'data': {'val': 9}}
>>> glom(target, (S(value = ('data', 'val')), S.value))
('data', 'val')
>>> glom(target,  (S(value = Auto(('data', 'val'))), S.value))
9
>>>  glom(target,  (S(value = T['data']['val']), S.value))
9

The assumption is the more common case is building an intermediate data structure. Also, T()-args, Invoke()-args, Assign(), and several default-args all have consistent behavior now whereas before some would treat their argument as a constant, and others would treat it as a spec; often there were special case cut-outs for T (and only T).

I'd be curious what your use case is where you are finding S handy. Usually if I have an intermediate step, it's just as easy to build a dict to hold temporary values "in parallel".

(
  {
     'val': ('data', 'val'),
     'other_val': ...
  },
  'val'
)

There's probably a gap in documentation that this came out of nowhere for you -- hopefully it wasn't too painful to figure out.

@DamianBarabonkovQC
Copy link
Author

I am unclear what you mean by build a dictionary to hold temporary values in parallel.

My more specific use case is something like in (v22 style):

target = ...
path = (
    S(id=("id",)),
    S(contract_type=("contract", "type"),
    ("receipts", [
        {
            "id": S.id,
            "contract_type": S.contract_type,
            "amount": ("amount",),
            "from": ("sender", "id"),
        }
    ])
)

Hope this clears up my exact usage more or less.

@mahmoud
Copy link
Owner

mahmoud commented Apr 26, 2023

Got it. A great use of scope if I've ever seen one.

To extrapolate your point, @DamianBarabonkovQC, and let me know if I'm wrong, but are you saying that when you use S, you typically expect to assign a dynamic value via spec (e.g., a tuple representing a path), as opposed to a literal (e.g., an actual tuple of strings)?

If so, I can see that point. If one were to want a literal during execution, there are several ways to get one, without using S. What do you think, @kurtbrose?

(btw, @DamianBarabonkovQC, what do you say to using the Path approach I described?)

@DamianBarabonkovQC
Copy link
Author

So I gave you an overly simplified example. Actually, my scope path is wrapped in a Coalese to handle any missing values with a MISSING. For example S(id=Coalesce("id", default=MISSING)). So I got this working by using the Spec keyword instead of the Path.

@kurtbrose
Copy link
Collaborator

path = (
    S(id=("id",)),
    S(contract_type=("contract", "type"),
    ("receipts", [
        {
            "id": S.id,
            "contract_type": S.contract_type,
            "amount": ("amount",),
            "from": ("sender", "id"),
        }
    ])
)

Oh, I see the challenge, you want to apply the outer values across each element of receipts. There's a few ways of doing that but none of them are cleaner than your current approach. I'd recommend switching to T in that case.

path = (
    S(id=T["id"], contract_type=T["contract"]["type"]),
    "receipts",
    [{
         "id": S.id,
         "contract_type": S.contract_type,
         "amount": "amount",
         "from": "sender.id",
    }]
)

I agree with your approach -- this is the type of case S is good for, where you need to cache an intermediate value that you'd otherwise lose track of.

Some other simplifications we can make:

  • one-item tuples are basically no-ops and can be removed
  • S() can accept any number of kwargs at once
  • nested tuples can be flattened out to the top level -- it's the same thing
  • tuple-of-strs can be dot joined

Those might be necessary in the original spec and have become redundant when you simplified it down here

@boonhapus
Copy link

In glom v22, I was able to assign a scope with a spec and that automatically got applied to the target. Version 23 breaks that.

Coming here to +1 the behavior change. Revisiting an old project in a new venv and spent 30mins debugging what the heck went wrong. 😅

I'd recommend switching to T in that case. ... this is the type of case S is good for, where you need to cache an intermediate value that you'd otherwise lose track of.

I swapped many implicit references to use T instead and things were great again.

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

4 participants