Allow varname to play with type annotation context #32

pwwang · 2020-12-18T17:10:53Z

Basically allow the following:

foo: Foo = Foo()
foo = Foo[int]()

pwwang · 2020-12-19T00:54:43Z

varname.py


    exet = executing.Source.executing(frame)
+    qualname = exet.code_qualname()
+    module = inspect.getmodule(frame)


@alexmojaki
Is there a better way for this with executing? Or this is already the best way.

I think this is fine. The alternative is to compare module.__file__ and frame.f_code.co_filename but I'm not actually sure if they always match.

alexmojaki · 2020-12-19T13:00:43Z

Why allow only passing a qualname without a module? Seems a bit risky...
What about passing the actual function object instead of a module and string? varname can then compare the code objects of the function and the frame. It would certainly feel like a nicer API, would catch errors sooner if users typed the name wrong. But if they pass a function that's been decorated it'll likely do the wrong thing, although if the decorator uses functools.wraps we could walk the chain of __wrapped__ and skip several layers automatically.
People often forget to pass a list and just pass one thing. You may want to check for that and either fix it automatically or raise a nice error.
I see you're not checking for unique qualnames in a module. Have you decided to assume it's best to just ignore all functions with that qualname in the module?

alexmojaki · 2020-12-19T13:09:55Z

Currently the code assumes that all the frames to ignore are grouped together in the stack. The user may combine caller and ignore in more complicated ways. Maybe caller is a high number to skip several unknown intermediate frames and also several known functions should be ignored but they're scattered in the stack more messily. I think you should iterate through the frames one by one and either skip because it's in ignore or reduce caller by one and continue until you reach 0.
Might also be good to ignore an entire module or filename. For example you could ignore all of varname.py and basically not use caller at all.

pwwang · 2020-12-21T17:13:27Z

@alexmojaki

Agree that it is risky to have only qualname. Though just to provide some convenience.
Passing a function directly that is decorated is absolutely a concern for this. Not sure if there is a way to detect if a function is decorated at AST level.
Agree
I am assuming that in the code, but want to add that kind of check, probably in the next commit, by borrowing the idea we have discussed.
So that's why I mentioned in the doc that if ignore is provided, then caller will work as a start point to scan the frames, yielding the frame that should not be ignored (not in ignore). But this may bring in some confusion. Maybe I have to doc it better.
This is definitely a better idea than passing a qualname only.

alexmojaki · 2020-12-21T18:02:07Z

2: The problem is that when someone provides a decorated function, we receive something like a locally defined wrapper. There's no way to get to the AST of the original function. I guess leave it as just the string but explain the reasoning in the docs. Otherwise some people might just think it's a bad API and pass MyClass.my_method.__qualname__ to avoid strings which turns out to be my_decorator.<locals>.wrapper.

5: I understand the doc, I don't think it's good behaviour.

pwwang · 2020-12-21T18:52:48Z

2: So, if I got all your points correctly, ignore will have 3 types of elements:

A module, of which the calls from inside will be ignored
A module and a qualname, pointing to the exact call from that module
A function, the exact call. If it is decorated, use "A module and a qualname"

5: Do you mean caller and ignore should be used mutually exclusive?

alexmojaki · 2020-12-21T19:17:50Z

2: No, let's not allow passing a function. It's not worth the risk. I briefly thought we could use varname style magic to check that the argument name and value match, e.g. in varname(ignore=Class.foo) it actually looks at that call and expects to find a function called Class.foo otherwise raises an error. It would be fun, but it's not worth the confusion.
5: No, the effects of caller and ignore should be able to be interleaved.

pwwang · 2020-12-21T22:02:57Z

5: If "caller is a high number to skip several unknown intermediate frames" and ignore is "several known functions should be ignored but they're scattered in the stack more messily", then how should decide which non-ignored frame to use in between (those scattered frames that should be ignored by ignore)?

alexmojaki · 2020-12-22T08:54:51Z

For example, this script works right now:

import varname

def foo1():
    return foo2()

def foo2():
    return foo3()

def foo3():
    return varname.varname(caller=3)

x = foo1()
assert x == 'x'

We'd want this to also work:

import varname

def my_decorator(f):
    def wrapper():
        return f()
    return wrapper

@my_decorator
def foo1():
    return foo2()

@my_decorator
def foo2():
    return foo3()

@my_decorator
def foo3():
    return varname.varname(caller=3, ignore="my_decorator.<locals>.wrapper")

x = foo1()
assert x == 'x'

The stacktrace looks like this:

  File "...", line 20, in <module>
    x = foo1()
  File "...", line 5, in wrapper
    return f()
  File "...", line 10, in foo1
    return foo2()
  File "...", line 5, in wrapper
    return f()
  File "...", line 14, in foo2
    return foo3()
  File "...", line 5, in wrapper
    return f()
  File "...", line 18, in foo3
    return varname.varname(caller=3)

It's an alternating mix of frames that should be ignored and frames that should be skipped by caller. Conceptually, first remove all the wrapper frames because of ignore:

  File "...", line 20, in <module>
    x = foo1()
  File "...", line 10, in foo1
    return foo2()
  File "...", line 14, in foo2
    return foo3()
  File "...", line 18, in foo3
    return varname.varname(caller=3)

Now caller=3 means 'skip the 3 frames foo3, foo2, and foo1', as it did in the original example.

pwwang · 2020-12-23T01:42:47Z

Decide not to check the uniqueness of qualnames from the same module. I thought about this uniqueness at the beginning because I didn't take into account the module. Users should take care of the duplicated qualnames from the same module themselves.

alexmojaki · 2020-12-23T07:37:56Z

Users should take care of the duplicated qualnames from the same module themselves.

How?

alexmojaki · 2020-12-23T10:49:39Z

varname.py

-            If `ignore` provided, this should be the stack where we start
-            to search the node that should not be ignored.
-        ignore: A list of qualnames or tuples of module and qualname that
+            Stacks are counted with the ignored ones being excluded.


There is one stack, consisting of many frames.

Is frame the right word? I am sort of confused. Are we ignoring frames or stacks?

Yes, we are ignoring and skipping frames.

alexmojaki · 2020-12-23T10:55:51Z

varname.py

+            # loot at next frame anyway at next iteration
+            frame_index += 1
+            module = inspect.getmodule(frame)
+            exect = executing.Source.executing(frame)


This is somewhat expensive, try to call it only when you need to get the node, i.e. when you return at the end. For just the qualname use executing.Source.for_frame(frame).code_qualname(frame.f_code). Even that should only be when you actually need the qualname.

alexmojaki · 2020-12-23T11:02:06Z

varname.py

+                _debug('Skipping frame from varname', frame)
+                continue
+
+            if module and module.__name__ in sys.builtin_module_names:


Here you check if module, later you don't. What does it mean if module is falsy?

Just because module.__name__ will raise AttributeError if module is None by inspect.getmodule. However, it's fine for other cases.

But you use module.__name__ again further down.

Correct, I should check it at L541 as well.

alexmojaki · 2020-12-23T11:04:14Z

varname.py

+                # if asyncio specified, asyncio.runners, asyncio.events, etc
+                # should be all ignored
+                modnames = module.__name__.split('.')[:-1]
+                if any(sys.modules['.'.join(modnames[:i+1])] in ignore
+                       for i, _ in enumerate(modnames)):
+
+                    _debug('Ignored', frame)
+                    continue


What about specifying (asyncio, 'function_qualname') where the function is actually in asyncio.runners? This prefix checking here only works for ignoring entire modules.

Then I kind of have to scan all qualnames in asyncio.runners to verify function_qualname is from it?

Suppose module.__name__ == 'foo.bar.spam' and exect.code_qualname() == 'func'. Then we need to check (foo.bar.spam, 'func') in ignore or (foo.bar, 'func') in ignore or (foo, 'func') in ignore.

Then what if there is a func in foo.bar or foo that one doesn't want to ignore?

Currently your code will ignore any function in foo.bar.spam if I set ignore=[foo]. I'm not entirely sure if this is a good idea but I can see the advantage so let's assume we keep that.

Also currently your code will not ignore the function func in foo.bar.spam if I set ignore=[(foo, 'func')]. I think that's inconsistent and confusing. You have this nice prefix wildcard system for entire modules but not functions.

I assume that if a pair (module and qualname) is received, one is ignoring an exact call (which can be located with the module (not the parent module) and qualname exactly). Otherwise, if a module is passed, I assume one wants to do some fuzzy ignoring (such as any calls from that module and submodules).

I was actually thinking about ignoring all frames from python standard libraries, instead of just modules from sys.builtin_module_names, since developers barely have a chance to touch them. However, I didn't find a good way to list all standard libraries. Most of the solutions are trying to walk through the library path.

In this way, developers don't have to worried about calls from libraries, such as typing if varname retrieving gets type annotations involved, asyncio if the retrieving is in async context, etc

But this is somehow too aggresive.

pwwang · 2020-12-23T17:11:15Z

Users should take care of the duplicated qualnames from the same module themselves.

How?

In such a case:

class Foo:
  ...

class Foo:
  ...

The first Foo is not retrievable at runtime. So I think users/developers should make sure that the qualname is referring to the retrievable calls. But I might have missed other situations where both objects are retrievable at runtime.

alexmojaki · 2020-12-23T17:45:07Z

The first Foo is not retrievable at runtime.

It might be.

class Foo:
    pass

x = Foo

class Foo:
    pass

assert x != Foo

For a more realistic example:

class Foo:
    @property
    def x(self):
        return 3

    @x.setter
    def x(self, value):
        pass

assert Foo.x.fget.__qualname__ == Foo.x.fset.__qualname__ == "Foo.x"
assert Foo.x.fget != Foo.x.fset

pwwang · 2020-12-23T17:47:18Z

The first Foo is not retrievable at runtime.

It might be.

class Foo:
    pass

x = Foo

class Foo:
    pass

assert x != Foo

For a more realistic example:

class Foo:
    @property
    def x(self):
        return 3

    @x.setter
    def x(self, value):
        pass

assert Foo.x.fget.__qualname__ == Foo.x.fset.__qualname__ == "Foo.x"
assert Foo.x.fget != Foo.x.fset

Makes sense. Then I will have to check the whole ignore list before I use it.

alexmojaki · 2020-12-23T17:54:02Z

The problem is I'm still not sure what the best behaviour is if duplicate qualnames exist. You could just ignore all the functions with that qualname, sometimes that will be perfect and very convenient. But if people only wanted to ignore one of the functions they might be surprised. Or you could raise an exception and not allow it at all, but now some things can't be ignored.

pwwang · 2020-12-23T18:10:06Z

The problem is I'm still not sure what the best behaviour is if duplicate qualnames exist. You could just ignore all the functions with that qualname, sometimes that will be perfect and very convenient. But if people only wanted to ignore one of the functions they might be surprised. Or you could raise an exception and not allow it at all, but now some things can't be ignored.

Here are the options:

Simply not allow a qualname in ignore to point to multiple objects. This is an easy and lazy option.
Allow but warn, and ignore all. Sort of responsible option.
Allow and ignore without warning. Confusing and probably unpredictable.

I tend to have option 2. Because in most cases, developers do not intend to specify a qualname pointing to multiple calls to ignore.

alexmojaki · 2020-12-23T18:19:40Z

You could raise an error or show a warning by default, but allow passing something extra, either another argument to varname or a third element of the tuple. It could be just a flag meaning "Yes I'm OK with duplicates just ignore them all" or it could be a function or code object to specify which one to ignore.

pwwang · 2020-12-23T18:43:47Z

You could raise an error or show a warning by default, but allow passing something extra, either another argument to varname or a third element of the tuple. It could be just a flag meaning "Yes I'm OK with duplicates just ignore them all" or it could be a function or code object to specify which one to ignore.

Sort of hesitating to add more arguments to varname. It's already too many. Unless it's implementing some key functionalities (ie. ignore).

Even there are chances to have qualnames referring to multiple calls, the chance to have all these calls involved along the varname retrieving is rare. I'd rather just show a warning, and leave it for the developers to solve it (simply wrap one of them and specify the qualname of the wrapper).

alexmojaki · 2020-12-23T19:05:28Z

Sort of hesitating to add more arguments to varname. It's already too many. Unless it's implementing some key functionalities (ie. ignore).

Yes, no solution is great. This is all very theoretical and unlikely to matter, it's just interesting to think and talk about. You can ignore me if you want.

Even there are chances to have qualnames referring to multiple calls, the chance to have all these calls involved along the varname retrieving is rare

Not sure what you mean. The problem exists even if the qualname appears only once in the stack. There doesn't have to be several calls involved at once. The problem is that two functions with the same qualname might exist in a file and the user intends to ignore just one but the other will also get ignored if it gets involved. Now as I'm writing, maybe I get it - you're saying that probably the extra functions will never get involved, as in they'll never indirectly call varname?

I'd rather just show a warning

Warnings can go unnoticed, and suppressing them can be annoying.

The Zen says "In the face of ambiguity, refuse the temptation to guess". Based on that I lean a little towards raising an exception. If someone has a problem they can raise an issue and we can learn about a real use case. Then we can change the behaviour without breaking compatibility.

pwwang · 2020-12-23T19:12:09Z

you're saying that probably the extra functions will never get involved, as in they'll never indirectly call varname?

Correct. I didn't mean those calls were to be involved in a single retrieving, but the chance is also rare for them to be involved in single retrieving individually and separately.

Based on that I lean a little towards raising an exception.

Convinced.

alexmojaki · 2020-12-23T19:19:56Z

This whole discussion is making me think that we might have it backwards. Instead a blacklist approach of ignoring and skipping frames, maybe it should be a whitelist approach, like "keep going until you reach one of these functions". Something like:

def foo1():
    return foo2()

def bar():
    return foo2()

def foo2():
    return foo3()

def foo3():
    return varname(caller=[foo1, bar])

x = foo1()
assert x == 'x'

But actually the more I think about it maybe not.

pwwang · 2020-12-23T19:27:52Z

I have thought about this way, too.

The problem is that this disallows others to wrap more layers around those functions. And sometimes, it is difficult to predict how it will be wrapped inside foo3.

Another sense is that, even though it is difficult to predict, the wrappers/intermediate calls are kind of grouped. For example, calls from typing and asyncio. So I think we are on the right track (to use ignore, combined with caller)

Or if it's deeply wrapped by a group of functions (say from the same module), and one of them is the end. It could be a disaster to list all those wrappers except for the end call.

I am actually also thinking that the name caller is a little bit confusing. It makes more sense to be called callee, or something like depth/frame.

pwwang · 2020-12-23T19:47:52Z

Maybe you were talking about allowing both depth and wrapper list for caller.

Then it'll be a fight of which one is more intuitive and easier to use between caller (with depth or wrapper list) and caller (with depth) + ignore.

alexmojaki · 2020-12-23T19:50:52Z

I just didn't bother to invent a new name. Let's just leave caller as is and forget the whitelist system.

…ck to _get_frame to avoid expense of converting each frame into an executing object.

pwwang · 2020-12-23T22:44:30Z

@alexmojaki Merge this, for now, to continue working on the preparation of v0.6. Feel free to open new issues or post here if you have further comments.

pwwang added 2 commits December 18, 2020 10:08

Allow assignment with type annontation for varname

776f432

Implement #33

0f00e90

pwwang changed the title ~~[WIP] Allow varname to play with type annotation context~~ Allow varname to play with type annotation context Dec 19, 2020

Fix for python3.6

003f5c7

pwwang commented Dec 19, 2020

View reviewed changes

Implement expected behavior for ignore and caller for varname.varname

c65a20d

Fix python3.6 for async.run in tests

6c05936

alexmojaki reviewed Dec 23, 2020

View reviewed changes

pwwang added 3 commits December 23, 2020 12:56

Add uniqueness check for qualname in ignore; Change _get_executing ba…

574cc32

…ck to _get_frame to avoid expense of converting each frame into an executing object.

Fix typo

b8d84a5

Put ignore list assertion into _check_qualname

d3f23f0

pwwang merged commit bd6b2d5 into master Dec 23, 2020

pwwang mentioned this pull request Dec 24, 2020

0.6.0 #39

Merged

4 tasks

alexmojaki mentioned this pull request Mar 8, 2021

Using ic in decorator/wrappers; showing alias function and parameters instead of actual function and passed parameters. gruns/icecream#57

Open

Allow varname to play with type annotation context #32

Allow varname to play with type annotation context #32

Uh oh!

Conversation

pwwang commented Dec 18, 2020

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

alexmojaki commented Dec 19, 2020

Uh oh!

alexmojaki commented Dec 19, 2020

Uh oh!

pwwang commented Dec 21, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

alexmojaki commented Dec 21, 2020

Uh oh!

pwwang commented Dec 21, 2020

Uh oh!

alexmojaki commented Dec 21, 2020

Uh oh!

pwwang commented Dec 21, 2020

Uh oh!

alexmojaki commented Dec 22, 2020

Uh oh!

pwwang commented Dec 23, 2020

Uh oh!

alexmojaki commented Dec 23, 2020

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

pwwang commented Dec 23, 2020

Uh oh!

alexmojaki commented Dec 23, 2020

Uh oh!

pwwang commented Dec 23, 2020

Uh oh!

alexmojaki commented Dec 23, 2020

Uh oh!

pwwang commented Dec 23, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

alexmojaki commented Dec 23, 2020

Uh oh!

pwwang commented Dec 23, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

alexmojaki commented Dec 23, 2020

pwwang commented Dec 21, 2020 •

edited

Loading

pwwang commented Dec 23, 2020 •

edited

Loading

pwwang commented Dec 23, 2020 •

edited

Loading

pwwang commented Dec 23, 2020 •

edited

Loading

pwwang commented Dec 23, 2020 •

edited

Loading