Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

'import six' can be after its first use #19

Open
daira opened this issue Aug 13, 2014 · 6 comments
Open

'import six' can be after its first use #19

daira opened this issue Aug 13, 2014 · 6 comments

Comments

@daira
Copy link
Contributor

daira commented Aug 13, 2014

$ cat test.py
u"artificial example"
import sys

$ python-modernize --six-unicode test.py
[...]
@@ -1,2 +1,3 @@
-u"artificial example"
+six.u("artificial example")
 import sys
+import six

which doesn't work because the import of six is after the first use.

(moved from mitsuhiko/python-modernize#20 )

@daira daira added this to the Release 0.3.1 milestone Aug 13, 2014
@daira daira added the bug label Aug 13, 2014
daira added a commit that referenced this issue Aug 28, 2014
@daira
Copy link
Contributor Author

daira commented Aug 28, 2014

@takluyver
Copy link
Contributor

Looks like the bug is actually in lib2to3.fixer_utils.touch_import.

daira added a commit that referenced this issue Aug 28, 2014
…udes the test for #19. refs #19, #26

Signed-off-by: Daira Hopwood <[email protected]>
@daira daira added the has test label Aug 28, 2014
@daira
Copy link
Contributor Author

daira commented Aug 28, 2014

This seems relevant: http://bugs.python.org/issue14282

@daira
Copy link
Contributor Author

daira commented Aug 28, 2014

Yes, I think adding imports after existing ones is wrong in general. Imports added by python-modernize should go:

  • after existing __future__ imports, if any
  • otherwise, after the module docstring, if any
  • otherwise, after any initial lines that are either blank or start with #.

(See also #18.)

@daira
Copy link
Contributor Author

daira commented Aug 28, 2014

According to grep -Rn --include='*.py' touch_import /usr/lib/python2.7/lib2to3/fixes (on Python 2.7.5ish), the fixers in lib2to3 itself that use touch_import are: fix_intern, fix_callable, fix_operator and fix_reduce. Of those, only fix_operator and fix_reduce are enabled by python-modernize. So if we changed our own uses of touch_import, we'd solve most of the problem, even though we wouldn't be solving it for a handful of cases in fix_operator (specifically importing collections.Sequence, collections.Mapping, or numbers.Number) and fix_reduce (importing functools.reduce), in the unusual situation that something that needs fixing is before another import. I think we can live with that.

daira added a commit that referenced this issue Sep 27, 2014
@takluyver takluyver mentioned this issue Oct 13, 2014
daira added a commit that referenced this issue Oct 14, 2014
daira added a commit that referenced this issue Oct 14, 2014
@daira
Copy link
Contributor Author

daira commented Oct 14, 2014

Huh, I can no longer reproduce this using the tests that reproduced it before (which are now on master). Bumping this out of the 0.4 release.

@daira daira modified the milestones: Release 0.5, Release 0.4 Oct 14, 2014
@daira daira self-assigned this Jan 18, 2015
daira added a commit that referenced this issue May 27, 2015
…udes the test for #19. refs #19, #26

Signed-off-by: Daira Hopwood <[email protected]>
daira added a commit that referenced this issue May 27, 2015
…udes the test for #19. refs #19, #26

Signed-off-by: Daira Hopwood <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants