update autoflake min version to 2.1.0

[Cheukting]

closes #88

[Cheukting]

Pass on my machine but not here, will have a look again tomorrow was passing the wrong args will fix it

[Cheukting]

    assert result == shed(
E   AssertionError: assert 'from os import (\n    waitpid,\n    waitstatus_to_exitcode,\n    walk,\n    write,\n    writev,\n)\n\nprint(\n    waitpid,\n    waitstatus_to_exitcode,\n    walk,\n    write,\n    writev,\n)\n' == 'from os import waitpid, waitstatus_to_exitcode, walk, write, writev\n\nprint(\n    waitpid,\n    waitstatus_to_exitcode,\n    walk,\n    write,\n    writev,\n)\n'
E     - from os import waitpid, waitstatus_to_exitcode, walk, write, writev
E     + from os import (
E     +     waitpid,
E     +     waitstatus_to_exitcode,
E     +     walk,
E     +     write,
E     +     writev,
E     + )
E       
E       print(
E           waitpid,
E           waitstatus_to_exitcode,
E           walk,
E           write,
E           writev,
E       )
____________

For some reason it is now multiline :-(

[Zac-HD]

If we look at the file, it's multiline before formatting so I think this is actually a good thing - this change means we're respecting the magic trailing comma when removing unused imports.

So if you run locally and commit that diff we'll be ready to merge!

[Cheukting]

This is super funky - first pass it's multiline, 2nd pass it's one line

[Cheukting]

So what result do we want? May be we need a passing the source code in shed twice for it to work?

[Cheukting]

It's isort that is being naughty (changing the multiline to a single line). I will have a look upstream then

[Cheukting]

To think of it, what we really want is isort to respect the already multiple-line import to remain multiple line, right? In this test case, autoflake (or other tools) remove the un-used import and now it become "too short" for multiple-line (according to sort). I think it makes sense from the perspective of each single tool (and it accidentally cause us problem as we are using them consecutively)

I don't see there is something very wrong with isort, except the fact that if you are only importing a few things it will put it in one line (and sort it) for you. I am not sure if it is a strong enough case to open an issue and PR upstream. We can of cause fix it on our end about the idempotent by running it twice.

I need your opinion on that @Zac-HD

[Zac-HD]

I think non-idempotence is a bug in isort, so constructing a minimal failing example which demonstrates that and reporting it upstream would be good. Until that's fixed, I don't think we can actually get a performance benefit here 😢

[Cheukting]

I have opened an issue at isort, I will have a look there to see if I can provide a PR. This one will have to wait I guess

[alessiamarcolini]

(setting next to @Cheukting now at EuroSciPy sprints :D) setting multi_line_output to 7 (NOQA) makes the tests pass, but is it what you want?

[Cheukting]

This is what it looks like after using the setting @alessiamarcolini recommended. @Zac-HD do we want to use it to by pass the upstream issues?

[Zac-HD]

If we can get this to work in Shed without needing upstream changes, that would be great!

[Zac-HD]

Handled in https://github.com/Zac-HD/shed/pull/105 🙂