Breaking behavior in latest release (1.0.1) about ddt HOT 9 CLOSED

I think as it is 2018 and whoever is using the lib now is probably using the newer approach anyway. I'm closing this for now.

from ddt.

Thanks for pointing this out. I didn't realise that was breaking existing behaviour.
I will see how to deal with that. In the meantime, pin your dependency to 1.0.0 if that gets you going, as I'm not sure when I can get some time to revert this/sort this out.

from ddt.

I've pinned it for now, so that should be enough to keep things rolling.

Thanks @txels !

from ddt.

What's the status of this?

from ddt.

@pradyunsg nothing has been done on this yet. Do you want to take a look?

from ddt.

AFAICT, this behavior shouldn't have changed in a patch-version bump. That was a bad mistake.

Since then, another minor version bump has occurred. I recommend the OP to port their tests now, since it's been a while since this change happened. Currently, some people might be using the newer versions (1.0.1+) and it won't be nice to suddenly break their code, intentionally.

It might be easy to write a decorator that handles both cases seamlessly for OP, if the expected number of arguments is 1 then pass a dictionary, if it's more, pass them as keyword arguments.

PS: I am indeed working on parts of this project. Something along the lines of #29, with some other ideas I have in my head. Once that's done, I would like to see all those changes come out as ddt 2.0.0.

I just wrote this decorator... It should work with Py2/3. It's untested though. You can possibly remove the conditionals in the start of the function if you are going to selectively apply only on your ddt test functions using the old format. Actually, that makes more sense. Because then you would clearly mark which test would need updating.

def adapt_if_needed(func):
    """ Adapt functions expecting ddt's old argument format to run with the newer one.
    """

    argspec = inspect.getargspec(func)

    # If it catches variable arguments, it's probably fine to ignore it
    if argspec.varargs is not None or argspec.keywords is not None:
        return func

    # Count required arguments
    required_args = len(argspec.args)
    if argspec.defaults:
        required_args -= len(argspec.defaults)

    # If it takes more than one argument, it's probably fine.
    if required_args > 1:
        return func

    # adapt for older version.
    @functools.wraps(func)
    def wrapper(**kwargs):
        func(kwargs)

    return wrapper

from ddt.

In all honesty, looking at PR #22 which is what seems to have caused this issue I would really suggest outright reverting #22 and then just telling people to break down the dictionary themselves. There's nothing stopping them from using a dictionary to manage how many entries they have and splitting it out doesn't really add much code to test.

Though if they really want to keep PR #22's functionality, perhaps it shouldn't be part of unpack but it's own unpack_dict to make that kind of functionality explicit.

@pradyunsg sorry...I'm not really seeing benefit to your proposed migration as it seems lacking in matching up arguments especially due to the assumption that a test that previously worked (as @Kuwagata exemplified) would only have 1 parameter, and thinking it through it would seem to me at least not an easy problem to try to determine how many parameters should be able to be matched up. So making the dict unpacking done via its own specific decorator would seem to be the most logical path. Anyone that has come to rely on the new methodology should be able to migrate easily to using a new decorator, which is what you're asking people that have been using the old methodology to do any way. And my guess is that people using the new method are probably fewer in number than people using the old method.

from ddt.

That sounds good too! Having a special decorator for dictionaries sounds like a nice idea. I would like a release period with the addition of the new decorator and a clear warning that the behavior shall change in the next minor version bump.

from ddt.

Bump. Could @txels make this change?

from ddt.