Fix/451 (WIP?)

[PGijsbers]

Worked on: #451

• Functions run_model_on_task and run_flow_on_task now allow both ordering of the first two arguments ((task, model) or (model, task) and (task, flow) or (flow, task), respectively).

• Added two simple unit tests which test that the normal output with swapped arguments still works.

There are some outstanding questions, see my last comment in #451. I propose we continue the discussion there.

I made an early pull request to check with CI (I have some issues with my local environment atm and don't have the time to fix it).

[PGijsbers]

note: AppveyorCI shouldn't work on this branch because it does not have the updated scripts.

[mfeurer]

Thanks a lot. It would be great if you could already swap the default order (i.e. the argument names and their documentation) and add a TODO which reminds us to deprecate the argument swapping. Also, you can most likely simplify the unit tests quite a lot by mocking out functions which are not related to swapping the arguments. Then you only need to check if the mock objects got called correctly.

[PGijsbers]

Adding a TODO where? Open an issue or a #TODO in the code? Also, I can just add a deprecation warning right now instead. It doesn't really take an effort, but what should the text be? Something like

warnings.warn("The old argument order (task, model) is deprecated and will not be supported in the future"
              "Please use the order (model, task).", PendingDeprecationWarning)

whenever arguments are passed by the old order?

Again I'm very sorry this took so long. I learned not to take up a new issues right before a vacation because there's always a lot to catch up to when you get back anyway ^^;

[mfeurer]

Yep, adding a #TODO as a comment was what I meant. And the deprecation warning would be great.

[mfeurer]

This looks great! Thanks a lot.

[mfeurer]

Could you please resolve the conflicts and check the failing tests?

[mfeurer]

Thanks! Test failures are due to missing tags on the test server. I will contact @janvanrijn about this.