Arg Mode

[kurtbrose]

Make T()-args, Invoke()-args, Assign(), and several default-args all have consistent behavior. Prior to this, different spec types 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).

[kurtbrose]

I think this is clearly on the right track -- Call() was already basically doing arg-mode, but doing it with a bunch of manual tap-dancing

[mahmoud]

Nice, yeah, there are a variety of precedents set in the existing specs. Invoke doesn't do that, though, does it? We ended up going with a more explicit approach because of the discomfort with Call's handling of this?

And for anyone following along (as well as for my own recollection), here's the bright line:

Should "subspec"-style arguments to specifier types respect the mode of the surrounding spec? Or should they be consistent across all specs and contexts?

This probably doesn't need to be democratized, and can be determined by comparing before-and-after with some existing cases, hence this PR.

[kurtbrose]

To unpack it a bit more -- in general subspecs should clearly follow Mode; the proposal here isn't to change that.

However, currently there are three kinds of values that give you different behavior:

• T, S, etc
• other specs
• non-spec python values

Depending on what you are doing "clearly" one or the other is the correct behavior. There's a lot of work trying to thread that needle:

• special cases for T in t_eval, assign, etc
• Literal() -> Val()
• Spec() spec
• FILL mode
• SKIP and STOP as literals

So, this is the latest in what has been a continuous attempt to sand down a rough edge.

What is the scope of applicability? These are some guidelines:

• specs and literal vals would both be useful
• there is special casing for T
• SKIP and STOP are used

These three criteria often overlap -- as was the case with Call().

As a simple motivating example:

S(a=T)  # stores current Target in S.a
S(a=(T, 1))  # evals T, tries to pass to 1 then fails
S(a=1)  # error: 1 is not a valid spec

with arg-mode

S(a=T)  # stores Target in S.a
S(a=(T, 1)) # stores (Target, 1) in S.a
S(a=1)  # stores 1 in S.a

[kurtbrose]

What are the downsides?

• forces mode understanding earlier -- I'm hopeful the examples will be so intuitive the most users won't really have to think about it, but this does mean that e.g. tuple, dicts mean different things in different parts of a spec even without explicit mode usage
• the worst case above is when using a spec that itself has subspecs, e.g. T[Coalesce('a', default='b')] -- since Coalesce would be called in arg mode, 'a' will evaluate to the literal 'a' instead of doing attribute access; however, currently that will do Target[Coalesce(...)], just taking the whole Spec as a literal and trying to fetch it as a key out of the target, which I think is clearly worse
• now for every subspec we need to decide if they are an "arg-mode" type subspec or a regular subspec -- but I think in practice going through the code base it will be pretty clear

The goal is making fewer special-case cutouts for T by having this new mode, and ending up with an overall simpler, more consistent, more comprehensible, more powerful system.

There's also some interesting interactions with M. We pulled M(subspec) and restricted it to only T. So, this is another special case of T. I don't think it is a good use case for arg-mode, because M() checks for truthiness -- which for all the data structures handled by arg-more will mean "is it empty", which will either be always true or always false.

[kurtbrose]

some more funny cases of arg-mode vs current behavior

S(a=[])  # in arg-mode, stores a new empty list each time; currently, errors
S(a=Val([]))  # stores the same empty list every time -- unlike arg-mode has same problem as python defaults
S(a=[T])  # in arg mode, stores [Target]; in Auto mode, if Target is iterable, stores list(Target)
S(a=list)  # in arg mode, stores `list` type; in Auto, list(Target)

[kurtbrose]

So, tentatively I think I've found the places that can benefit:

• S()/Let() assignment (side note -- are we deprecating Let?)
• default= on Coalesce, Check, Match, Optional, Switch, And/Or
• T[], T(), T.

On balance, I think the changes to the API make things simpler.

The biggest potential source of confusion is str-as-constant vs str-as-spec in the case of S() -- since this feature has only been out for ~ 6 weeks, I think maybe we can get out ahead of it and change the behavior without ruffling too many feathers.

[kurtbrose]

definitely the changes that were required to unit tests are interesting

[kurtbrose]

oops I missed Assign() -- the val argument there would probably benefit from arg-mode

THAT API has been around for a long time, so changing the behavior of Assign(Spec(...)) is probably a bad idea; will need to think about how to allow arg_val(Spec()) to NOT change the Mode for child specs; Group has a similar challenge where ideally the Mode would project down through constants, but when hitting a Spec class would revert to the outer enclosing Mode; I didn't pursue it further with Group b/c it seems to be a rare use case -- with it popping up where maybe I can think of a generic solution

inside Assign.glomit():

        if type(self.val) is Spec:
            val = scope[glom](target, self.val, scope)
        else:
            val = self.val

that special casing is what we can get rid of with arg-mode :-)

[kurtbrose]

auto mode dict keys could also benefit (another place of T special casing)

[kurtbrose]

found kind of an interesting corner case with trace rendering; locally all tests are green now

I think it is ready to go

[kurtbrose]

maybe I should add stuff to the docs though?

[codecov[bot]]

Codecov Report

Merging #196 (0c7c97e) into master (032f252) will increase coverage by 0.06%. The diff coverage is 100.00%.

@@            Coverage Diff             @@
##           master     #196      +/-   ##
==========================================
+ Coverage   98.38%   98.44%   +0.06%     
==========================================
  Files          27       27              
  Lines        4338     4509     +171     
  Branches      710      784      +74     
==========================================
+ Hits         4268     4439     +171     
  Misses         46       46              
  Partials       24       24              

Impacted Files	Coverage Δ
glom/test/test_basic.py	100.00% <ø> (ø)
glom/test/test_error.py	99.53% <ø> (-0.02%)	:arrow_down:
glom/core.py	98.68% <100.00%> (+0.16%)	:arrow_up:
glom/matching.py	100.00% <100.00%> (ø)
glom/mutation.py	99.33% <100.00%> (-0.01%)	:arrow_down:
glom/test/test_match.py	100.00% <100.00%> (ø)
glom/test/test_mutation.py	98.11% <100.00%> (+0.06%)	:arrow_up:
glom/test/test_path_and_t.py	99.06% <100.00%> (+0.04%)	:arrow_up:
glom/test/test_scope_vars.py	100.00% <100.00%> (ø)

:mega: Codecov can now indicate which changes are the most critical in Pull Requests. Learn more