-
Notifications
You must be signed in to change notification settings - Fork 47
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
unicode support in function_key_generator on Python2 #94
Comments
Antoine Bertin (diaoul) wrote: I also have this issue and agree with the proposed solution. |
Michael Bayer (zzzeek) wrote: would be curious to know @morganfainberg 's take here also. |
Morgan Fainberg () wrote: My concern with making this change is related to the backend libraries. I'm looking into how this ends up getting handled in the variety of cases and if they handle the byte_str differently than the text_string. We are dealing with what we are potentially sending to the memcached/redisserver/etc as the cache key, and in some libraries we might be scraping by simply based upon the fact that we have always been passing a c-string (byte_str) instead of the alternative. An alternative would be to force a .encode() as the default. With all of that said, I am not outright opposed to this change. I am slightly concerned that we're changing a default behavior (which I tend to prefer to save for a major version bump). If we are changing this, I prefer not needing to be clever about encoding. |
we should probably do it, ive added an 0.7 milestone to have the version bump |
Migrated issue, originally created by Wolfgang Schnerring (wosc)
function_key_generator
definesto_str=compat.string_type
by default, which isstr
on Python2 (so breaks when the function gets non-ascii arguments).The tests don't catch this, because they explicitly pass
to_str=compat.text_type
, which does the right thing.Are the backwards-compatibility concerns, or could the default be changed to
text_type
, which would certainly be less... surprising, out-of-the-box.The text was updated successfully, but these errors were encountered: