input_munger error handling and refactor

[Drblessing]

What was wrong?

The input_munger function had no friendly error output, as well as readability issues and verbose logic. This PR also makes it faster in my local testing. We can reduce the input_munger function down to a single line with:

return functools.reduce(lambda args, munger: munger(module, *args, **kwargs), self.mungers, args)

if desirable.

How was it fixed?

• Refactor pipe the munged_inputs to a for loop.
• Catch TypeErrors while mungers are munging input parameters.
• Remove _munger_star_apply function.

Todo:

• Add entry to the release notes

Cute Animal Picture

[fselmo]

Hey @Drblessing, thanks for the PR. I love the refactor here. I'm not exactly sure what the TODO really was there for though. It seems like we would want to just let the munger raise its exception with the original message? I know you can trace back to it in the traceback but I'm not seeing the benefit in raising a new TypeError with a more broad message from the old TypeError that has a more clear message to begin with.

Also, TypeError isn't the only exception that can be raised from a munger. At the moment I see at least one instance of a ValueError. Then we have to keep catching those as well. I'd be happy with just this refactor and removing the TODO to be honest.

Curious on your thoughts here. I may very well be missing something.

[Drblessing]

Hey @fselmo thank you for the compliment. I agree with everything you stated.

Regarding the TODO, I initially thought wrapping the exception could provide a uniform error handling mechanism for all munger related errors. However, I do see your point about the original exception potentially having a more clear and informative message. Raising a broad TypeError might indeed obscure the specifics of the error and make it harder to debug, and miss other ValueError.

Given these insights, I'd be more than happy to roll back the exception handling change and focus solely on the refactor. Removing the TODO as you suggested also sounds like a clean approach.

Would the single-line solution work, assuming it passes the tests?

return functools.reduce(lambda args, munger: munger(module, *args, **kwargs), self.mungers, args)

Let me know if that works for you and I'll make the necessary updates!

[fselmo]

Let me know if that works for you and I'll make the necessary updates!

That sounds like a good plan to me 👍🏼. Thanks!

[fselmo]

Thanks again @Drblessing 👍🏼

[Drblessing]

Thank you @fselmo ! 😄