Don't add 'absolute_import' futures indiscriminately #165
Conversation
I'm looking at the test failures now. Some are a matter of updating the test expectations, but others involve idempotency issues that the blanket 'absolute_import' adds hid previously. I suspect the first pass adds imports (e.g., 'six') where there were none before (thus the 'import' filter doesn't trigger to add the 'absolute import' future on that pass). The second pass, now with an import, triggers the 'import' filter. While it is good to have idempotency when possible, this is an example of where two filters will interact in a way that requires a second pass to fully realize. Edit: It appears almost all the errors may be idempotency issues related to |
Updated tests accordingly
Idempotence is a bigger concern for @daira and @brettcannon than it is for me (based on e.g. #44 and #54), so I'd like them to take a look at this. I suspect that idempotence might be why we're adding |
My argument is that forcing single-pass idempotence by adding To reiterate, my fix is still idempotent, it just avoids unnecessary out-of-band code changes that cause side-effects whilst trying to side-step the fact that interdependent fixers require an extra internal pass. |
Thanks. Idempotence was never something terribly important to me, but it's something that I know other people spent a lot of time on, so I'd like them to weigh in before I merge any changes that affect it. |
I don't care anymore. |
I value idempotence - the worst situation is code that never stops mutating, and the second worse is not knowing when it should stop mutating. I'm satisfied that idempotence is still maintained and that this behavior should be stable within the bounds of the tests available, as well as the useful lifetime of this tool. |
I think @daira was the most keen on idempotence previously, so I'll still give her a few days to comment on this. If we don't hear anything, then I think we can carry on. |
Just checking in as it's been a week now. And note again that idempotence is being preserved here, without the previously out-of-band application of the When this is merged, can we get a point release please? |
Thanks for your patience, I'll merge this now. |
Released 0.6.1. |
Thank you! |
Currently the 'absolute_import' future is added anywhere a non-future import happens. This branch removes that side-effect. The existing 'import' fixer has a very similar effect and can be directly enabled or disabled by the user.
Fixes #163 #142