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

Allow ic to be used as a decorator #32

Open
skorokithakis opened this issue May 15, 2019 · 18 comments · May be fixed by #73
Open

Allow ic to be used as a decorator #32

skorokithakis opened this issue May 15, 2019 · 18 comments · May be fixed by #73

Comments

@skorokithakis
Copy link

skorokithakis commented May 15, 2019

I find that I want to inspect the arguments and return value of a function whenever it's called, but right now I have to go to every line that calls it and add ic() around the call. Moreover, that doesn't even work because it's a Flask function and something messes up.

It would be much better if I could do something like:

@ic
def foo(bar, baz):
    return bar + baz

And get the arguments and return value printed every time the function got called.

@gruns
Copy link
Owner

gruns commented May 15, 2019

Wonderful idea.

The space for a decorator is vast, as the decorator can take all manner of
parameters.

A simple @ic decorator, sans parameters, that prints all invocation pairs of
argument(s) and return value(s) is a great start.

Do you have bandwidth to tackle this? If not, I'll get around to it, but it will
be some time.

@skorokithakis
Copy link
Author

I might do, although I haven't looked at the code and don't know how ic will be able to tell it's being used as a decorator versus ic(somefunc). Do you have any ideas there?

@gruns
Copy link
Owner

gruns commented Jul 13, 2019

Decorators just takes a function and return another function. So ic() could
just check if its first argument is a function and go from there.

class IceCreamDebugger:
    def __call__(self, *args):
        if args and callable(args[0]):  # Decorator use detected.
            # Do decorator magic here.
        else:
            # Normal behavior, like ic('foo').

@alexmojaki
Copy link
Collaborator

But if I write ic(x) and x happens to be a function I'll be very confused when I don't see ic| x: ... in the logs.

@gruns
Copy link
Owner

gruns commented Jul 13, 2019

Excellent catch @alexmojaki! Silly, trivial folly on my end. No morning ☕
on weekends :).

The two viable approaches:

  1. Add a special keyword to ic(), like
@ic(decorator=True)
def foo():
    ...

But this approach is bad; magic values like decorator are bad design. For
example, what happens if one wants to provide a variable named decorator to
ic() and see its value?

  1. Have a separate, distinct decorator function. like
from icecream import icDecorator

@icDecorator
def foo():
    ...

This is the most robust, independent approach. But it doesn't reuse ic(). The
name of the decorator here is important to be short but clear.

@alexmojaki
Copy link
Collaborator

  1. ic(decorator) and ic(decorator=True) would resolve to different arguments inside ic and can be distinguished just fine.
  2. I think a separate decorator should be a method of ic so that if the user has already written from icecream import ic they can just write @ic.something without any more importing.
  3. It should be possible to do some magic of the style icecream already does to determine if it's being used as @ic. But it's possible the user will want to do decorate manually like foo = ic(foo). Is that a concern?

@skorokithakis
Copy link
Author

I think @ic.decorate is a good compromise and not really that much harder to type. I think that's the best of all worlds.

@alexmojaki
Copy link
Collaborator

It should probably have a more descriptive name like log_calls, for readability and in case this library every provides other decorators.

@skorokithakis
Copy link
Author

Yes, I didn't mean "decorate" should be the actual name, just a method on ic.

@WAY29
Copy link

WAY29 commented Jul 8, 2020

Has this issue been solved?Can I use ic as a decorator?

@pjz
Copy link

pjz commented Mar 30, 2021

Obviously this should be called ic.wrap ...

@skorokithakis
Copy link
Author

Not ic.sandwich?

@salabim
Copy link

salabim commented Mar 30, 2021

I don't see why you would need a separate method to use ic as a decorator.
The package ycecream (https://github.com/salabim/ycecream) package does it all with standard function (in this case y instead of ic). Why can't IceCream something similar?
Also, ycecream can be used as a context manager. Why not IceCream?

@alexmojaki
Copy link
Collaborator

alexmojaki commented Mar 30, 2021

I don't see why you would need a separate method to use ic as a decorator.
The package ycecream (https://github.com/salabim/ycecream) package does it all with standard function (in this case y instead of ic). Why can't IceCream something similar?

I addressed this idea:

3. It should be possible to do some magic of the style icecream already does to determine if it's being used as @ic. But it's possible the user will want to do decorate manually like foo = ic(foo). Is that a concern?

Decoration doesn't always use @. I've often written code like this to dynamically decorate many functions:

import inspect
from ycecream import y

class C:
    # @y
    def foo(self):
        return 1

    def bar(self):
        return 2

for name, method in inspect.getmembers(C, inspect.isfunction):
    decorated = y(method)
    setattr(C, name, decorated)

C().foo()

The expected output, which I get if I use @y, is:

y| called foo(<__main__.C object at 0x7fd101453c90>)
y| returned 1 from foo(<__main__.C object at 0x7fd101453c90>) in 0.000040 seconds

The actual output is just:

y| method: <function C.bar at 0x7fd101e2b830>
y| method: <function C.foo at 0x7fd101e35f80>

Even assuming just @y, this kind of magic is delicate and should be done with care. This code:

from ycecream import y

@y(
    prefix="pre",
    show_time=True,
    show_exit=False
)
def add2(i):
    return i + 2

add2(3)

works in Python 3.8 and 3.9 but breaks in 3.7:

pre#3 @ 19:15:09.745189
Traceback (most recent call last):
  File "/home/alex/.config/JetBrains/PyCharm2020.3/scratches/scratch_1218.py", line 6, in <module>
    show_exit=False
TypeError: 'NoneType' object is not callable

Also, ycecream can be used as a context manager. Why not IceCream?

Because no one has asked for it? There's already plenty of libraries and snippets on StackOverflow and beyond that do this easily in a few lines of code. Sure it might be nice to have in icecream but it doesn't surprise me that there's no demand. If you think icecream should have it, make a PR.

@salabim
Copy link

salabim commented Mar 30, 2021

You are addressing some very edge case that ycecream doesn't address properly. If you would like to have that changed, please make it an issue on the ycecream GitHub page.

With repect to PRs for IceCream: I have all the functionality that I need already in ycecrean, so I have absolutely no need for that in IceCream. But maybe your users have?

@alexmojaki alexmojaki linked a pull request Apr 2, 2021 that will close this issue
@alexmojaki
Copy link
Collaborator

alexmojaki commented May 9, 2021

I've implemented this in #73 with an upgrade to executing that detects decorators reliably. Decorating dynamically without the @ syntax requires explicitly calling ic.decorate(func). Could I get some opinions/votes on desired naming and features?

@gruns
Copy link
Owner

gruns commented May 12, 2021

I've implemented this in #32 with an upgrade to executing that detects decorators reliably.

🎉!

Could I get some opinions/votes on desired naming and features?

does the new executing hotness support:

  1. @ic without args, eg for a default decorator with the fewest characters
@ic
def foo():
    pass
  1. @ic(...) with args, eg
@ic(timer=True)
def foo():
    pass

and/or 3. @ic.method(...) with args, eg for flexibility to have have multiple decorators

@ic.timer(utc=True)
def foo():
    pass

@ic.trace
def foo():
    pass

if all three, we've got maximum surface area to decide the best API @

@alexmojaki
Copy link
Collaborator

Yes, you can put any expression after @.

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

Successfully merging a pull request may close this issue.

6 participants