Allow extra arguments to ic call

[Joaq33]

This change allows you to add extra keyworded arguments to your ic function call. This solves a problem that I faced when I wanted my list prints to be customized, but ic didn't allow me to change the argToStringFunction's optional arguments. As an example, the default function that works in the background to format the input, pprint.pformat, could accept the optional arguments such as compact, but ic doesn't handle those.

Code:

l = [111111, 222222, 333333, 444444, 555555, 666666, 777777, 888888, 999999, 101010, 111111]

ic(l, compact=True) # this would fail without this commit
ic(l)

Output:

ic| l: [111111, 222222, 333333, 444444, 555555, 666666, 777777, 888888, 999999, 101010,
        111111]
ic| l: [111111,
        222222,
        333333,
        444444,
        555555,
        666666,
        777777,
        888888,
        999999,
        101010,
        111111]

Also, this supports the usage of kwargs in user created argToStringFunction

*I hope you like it, this is my first open source contribution

[Joaq33]

@gruns can you take a look at this?

[Joaq33]

I still think this is an useful non-intrusive addition, I still apply the pr every time I install the package Can you check it for a merge? @Jakeroid

*if you want extra explanations or use cases, I'm available to provide

[gruns]

hey @Joaq33! thank you for the incessant pokes and follow-ups! theyre always helpful 🙏

lets back up a bit and see if this is the best solution to the problem here. instead of adding **kwargs to ic() and passing those kwargs through to self.argToStringFunction(), which params do you end up adding yourself? like compact=True. any others? and how do they change the output?

why? because i think the better solution here is to add those arguments, explicitly, to ic(). eg add compact= as a param which affects ic()'s output

why? because it strikes me as poor API design to accept arbitrary kwargs that may, or may not, have meaning. for example, if you call ic(..., someMadeUpParam='lolsup') and no error is thrown, that's unepxected. interfaces should be as explicit as possible. this is especially true for public, long-lived APIs. like ic()

[Joaq33]

You are right, I revisited this pr and it's wrong. I will upload another tested version in the future based on the same idea. The idea is to make these optional kwargs interact directly with the underlying pprint.pformat (if no custom formatter function is provided), so, any call with an undefined kwarg in that function will raise an error.

For example:

ic(l, someMadeUpParam='lolsup')

will output:

TypeError: pformat() got an unexpected keyword argument 'someMadeUpParam'

and it's traceback.

---

Here is another example but now using a custom defined outputFunction with optional arguments, I will include it as a test too

def testOutputFunctionKwargs(self):
     lst = []
     def appendTo(s, optional=False):
         if optional:
             lst.append(s + 'with_option')
             return
         lst.append(s)
     with configureIcecreamOutput(ic.prefix, appendTo):
         with captureStandardStreams() as (out, err):
             ic(a)
     assert not out.getvalue() and not err.getvalue()
     with configureIcecreamOutput(outputFunction=appendTo):
         with captureStandardStreams() as (out, err):
             ic(b, optional=True)
     assert not out.getvalue() and not err.getvalue()
     pairs = parseOutputIntoPairs(out, '\n'.join(lst), 2)
     assert pairs == [[('a', '1')], [('b', '2with_option')]]

This will allow conditional printing, log levels, and other conditionals with the same ic configuration. Regarding your inquiry, I now see what you mean. I'll include specific error handling as well to improve maintainability.

Thank you for your attention and feedback.

[ruudvanderham]

Wouldn't it be better to check (with inspect.Signature) if all of the kwargs are indeed in the signature. If not, raise an exception at configure time. Best of both worlds ...

[gruns]

@ruudvanderham can you provide an example of what you mean? how would this work if both functions - ic() and argToStringFunction() - just accept kwargs and process them internally?

@Joaq33 another option would be to add a single new param to ic(), like argToStrKwArgs={...}’, that would accept a dict that could be passed through to kwargs of self.argToStringFunction(). that avoids making ic()'s arguments accept every keyword argument without error

so zooming out, the problem to address here is

1. you can customize the argToStringFunction with ic.configureOutput(argToStringFunction=...)
2. but then, after setting your argToStringFunction, you cant pass any parameters to it at call time when invoking ic()

correct? i want to make sure we're on the same page with the problem so we can arrive at the best solution 🙂

[salabim]

Rather than answering from @ruudvanderham, I answer from my other GitHub account (@salabim).

Can you provide an example of what you mean? how would this work if both functions - ic() and argToStringFunction() - just accept kwargs and process them internally? The idea is that the given argToStringFunction should not accept kwargs, but instead lists all arguments correctly (like pprint). If that's the case, ic could easily check whether a given kwarg is accepted by the print function. And if not, raise an exception. Does that make sense?

[gruns]

hey @salabim! yep! helpful. thank you! 🙂

that said, I'm not a fan of this approach because it places an arbitrary requirement on the argToStringFunction() function to:

1. only use explicit keyword params, eg , foo=, blah=, ..., and
2. not use kwargs, eg no , **kwargs)

why? because kwargs can't be checked explicitly for parameter conformance with inspect.Signature

this, in turn, would be an extra requirement users of icecream would have to know, and remember, when adopting a custom argToStringFunction. in short? its surprising behavior

and, in turn, I'm not a fan of this design

additionally, this design doesnt 'scale' because there are multiple configurable functions

ic.configureOutput(prefix, outputFunction, argToStringFunction, includeContext, contextAbsPath)

and perhaps more in the future. notably among them: outputFunction(), which you could also desire to pass custom parameters through ic()

so if you add a parameter like ic(foo, format=True) to ic(), should this param be sent to argToStringFunction() or outputFunction()? it's not clear

so! what's a better design. one design i dont like would be to add separate kwargs= for argToStringFunction() and outputFunction() to ic(). example:

ic(foo, argToStrFnParams={'limitColors': False}, outputFnParams={'format':True})

but this is crazy verbose. so also not a fan of this

another, less verbose approach, would be to have a single kwargs= param in ic(), like ic(foo, **kwargs), and this kwargs dict would be passed to both argToStringFunction() and outputFunction() to be processed. so all kwargs provided to ic() would be sent to both argToStringFunction() and outputFunction(), and it'd be up to those functions to detect and then process any relevant params

downside of this? cant check for explicit params with inspect.Signature and thus nonsense, unused params can be provided and no errors thrown

cc @Jakeroid