Always replace unicode() with six.text_type
#38
Conversation
|
Fixed the libmodernize.fixes.init problem and created #39 for discussion. |
|
This looks good, but I wonder if we should get rid of the check that it's called. The other main use for the name unicode is in isinstance checks, where replacing it with text_type does make sense. Or there could be a separate fix that checks for isinstance(x, unicode). |
|
Patch updated to substitute all found instances of |
|
Changes Unknown when pulling 0c9efdc on brettcannon:unicode_callable into * on python-modernize:master*. |
|
|
||
| def transform(self, node, results): | ||
| touch_import(None, u'six', node) | ||
| return Name(u'six.text_type', prefix=node.prefix) |
There was a problem hiding this comment.
I would lean towards doing from six import text_type and then using the name text_type, because then it's easier to change if you're using your own compatibility code instead of six.
There was a problem hiding this comment.
This would go against the current practice in the other fixers of only doing from six import ... statements when doing global overrides, e.g. for map. Plus not using 'six' is such a pain even I gave up trying after all those issues I filed about it.
But if your leanings are more requirements I'll change the code (I obviously have a personal aversion to importing things directly from modules which is why I'm pushing back a little =).
There was a problem hiding this comment.
That's fair enough. It's only a leaning, not a requirement, and I accept your leanings in the other direction.
There was a problem hiding this comment.
Thanks for understanding. =)
Always replace `unicode()` with `six.text_type`
No description provided.