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

instead of xxx_todo_changeme, use a name based on the original variables #24

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

Comments

@daira
Copy link
Contributor

daira commented Aug 13, 2014

E.g. instead of

-        def _got_root((root, path)):
+        def _got_root(xxx_todo_changeme):
+            (root, path) = xxx_todo_changeme

use

-        def _got_root((root, path)):
+        def _got_root(root_and_path):
+            (root, path) = root_and_path

Also, the X_and_Y convention could be used in cases like this:

-        d.addCallback(lambda (root, path):
-                      self._get_or_create_directories(root, path, metadata))
+        d.addCallback(lambda root_path:
+                      self._get_or_create_directories(root_path[0], root_path[1], metadata))

Here the generated name root_path is inappropriate because that would mean something like "path of the root", as opposed to a (root, path) pair. root_and_path would be better.

(moved from mitsuhiko/python-modernize#15 )

@takluyver
Copy link
Contributor

I believe the variable name xxx_todo_changeme comes from 2to3, not libmodernize, so we can't easily fix it.

@daira daira added this to the Release 0.5 milestone Sep 27, 2014
@daira
Copy link
Contributor Author

daira commented Jan 10, 2015

The second problem (root_path vs root_and_path) is just in the implementation of the tuple_name function of https://hg.python.org/cpython/file/4d4a9094bdb0/Lib/lib2to3/fixes/fix_tuple_params.py . The first is because at line 68, self.new_name is called without an argument (compare with line 124).

I think there is enough motivation to fork this code in order to fix this issue and #15.

@daira daira self-assigned this Jan 10, 2015
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