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?

[Zac-HD]

Looks great, if only the tests were passing!

I think we need to work out whether it's isort or autoflake (or potentially another tool) causing this instability, and then report upstream that it's not idempotent. Hopefully they can then fix that, and we'll then depend on the fixed release...

[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 #105 🙂