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

Lazily generate US_STATES, STATE_CHOICES, and USPS_CHOICES #272

Merged
merged 1 commit into from
Jan 3, 2017
Merged

Lazily generate US_STATES, STATE_CHOICES, and USPS_CHOICES #272

merged 1 commit into from
Jan 3, 2017

Conversation

PaulSD
Copy link
Contributor

@PaulSD PaulSD commented Dec 18, 2016

Commit 4369237 introduced lazy translations for US state names.
However, US_STATES, STATE_CHOICES, and USPS_CHOICES are sorted at import
time based on the translated state names. This causes two problems:

  1. If localflavor.us is imported during Django initialization, then an
    exception will be thrown because translations cannot be made during
    Django initialization.
    (See Error - "The translation infrastructure cannot be initialized before the " #203)
  2. When the run-time translations are different than the import-time
    translations, US_STATES, STATE_CHOICES, and USPS_CHOICES are not
    re-sorted based on the run-time translations, so those lists may be
    mis-sorted.

This commit corrects these problems by lazily sorting US_STATES,
STATE_CHOICES, and USPS_CHOICES. (Fixes #203)

Unfortunately, while this works as expected in most cases, the lazy
evaluation may not work as expected in every case due to internal tuple
optimizations in the Python interpreter. For example,
US_STATES + ('XX', _('Select a State')) works as expected, but
('XX', _('Select a State')) + US_STATES throws
'TypeError: can only concatenate tuple (not "proxy") to tuple'
because Python concatenates the tuples in C code without making any
Python code calls to the US_STATES object. To work around this, you can
use a slice index to force the necessary Python code calls:
('XX', _('Select a State')) + US_STATES[:]

  • Add an entry to the docs/changelog.rst describing the change.

  • Add an entry for your name in the docs/authors.rst file if it's not
    already there.

  • Adjust your imports to a standard form by running this command:

    `isort --recursive --line-width 120 localflavor tests`
    

@benkonrath
Copy link
Member

Thanks for finding a solution for this problem. I think it makes sense to get this into the next release (which I'm planning to do soon).

The workaround for the optimization issue in the Python interpreter should be part of the documentation. It doesn't have to be detailed - just the workaround would be ok. The documentation could be included with each tuple where the problem could occur or as general documentation at the top of the Data section. I'm not sure off hand if it's possible to add general documentation to the Data section. In any case, I'll leave it to you to decide what is best.

@@ -51,6 +51,8 @@ Other changes:
- Ensure the migration framework generates schema migrations for model fields that change the max_length
(`gh-257 <https://github.com/django/django-localflavor/pull/257>`_). Users will need to generate migrations for any
model fields they use with 'makemigrations'.
- Lazily generate US_STATES, STATE_CHOICES, and USPS_CHOICES
(`gh-272 <https://github.com/django/django-localflavor/pull/272>`_).
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could you also add a link to issue 203? You can see how to add 2 links in other changelog entries. Thanks.

@PaulSD
Copy link
Contributor Author

PaulSD commented Dec 30, 2016

Updated

@benkonrath
Copy link
Member

Thanks for the quick turnaround. The documentation isn't being picked up by Sphinx. For instance, US_STATES looks like this in the generated HTML:

localflavor.us.us_states.US_STATES = u"(('AK', <django.utils.functional.__proxy__ object at 0x32ad890>), ('AL', <django.utils.functional.__proxy__ object at 0x3384190>), ('AR', <django.utils.functional.__proxy__ object at 0x3384210>), ('AZ', <django.utils.functional.__proxy__ object at 0x33841d0>), ('CA', <django.utils.functional.__proxy__ object at 0x3384250>), ('CO', <django.utils.functional.__proxy__ object at 0x3384290>), ('CT', <django.utils.functional.__proxy__ object at 0x33842d0>), ('DC', <django.utils.functional.__proxy__ object at 0x3384390>), ('DE', <django.utils.functional.__proxy__ object at 0x3384310>), ('FL', <django.utils.functional.__proxy__ object at 0x3384410>), ('GA', <django.utils.functional.__proxy__ object at 0x3384490>), ('HI', <django.utils.functional.__proxy__ object at 0x32ad910>), ('IA', <django.utils.functional.__proxy__ object at 0x3384650>), ('ID', <django.utils.functional.__proxy__ object at 0x3384510>), ('IL', <django.utils.functional.__proxy__ object at 0x3384590>), ('IN', <django.utils.functional.__proxy__ object at 0x3384610>), ('KS', <django.utils.functional.__proxy__ object at 0x33846d0>), ('KY', <django.utils.functional.__proxy__ object at 0x3384750>), ('LA', <django.utils.functional.__proxy__ object at 0x33847d0>), ('MA', <django.utils.functional.__proxy__ object at 0x3384950>), ('MD', <django.utils.functional.__proxy__ object at 0x33848d0>), ('ME', <django.utils.functional.__proxy__ object at 0x3384850>), ('MI', <django.utils.functional.__proxy__ object at 0x33849d0>), ('MN', <django.utils.functional.__proxy__ object at 0x3384a50>), ('MO', <django.utils.functional.__proxy__ object at 0x3384b50>), ('MS', <django.utils.functional.__proxy__ object at 0x3384ad0>), ('MT', <django.utils.functional.__proxy__ object at 0x3384bd0>), ('NC', <django.utils.functional.__proxy__ object at 0x3384f50>), ('ND', <django.utils.functional.__proxy__ object at 0x3384fd0>), ('NE', <django.utils.functional.__proxy__ object at 0x3384c50>), ('NH', <django.utils.functional.__proxy__ object at 0x3384d50>), ('NJ', <django.utils.functional.__proxy__ object at 0x3384dd0>), ('NM', <django.utils.functional.__proxy__ object at 0x3384e50>), ('NV', <django.utils.functional.__proxy__ object at 0x3384cd0>), ('NY', <django.utils.functional.__proxy__ object at 0x3384ed0>), ('OH', <django.utils.functional.__proxy__ object at 0x32ad090>), ('OK', <django.utils.functional.__proxy__ object at 0x32ad110>), ('OR', <django.utils.functional.__proxy__ object at 0x32ad190>), ('PA', <django.utils.functional.__proxy__ object at 0x32ad210>), ('RI', <django.utils.functional.__proxy__ object at 0x32ad290>), ('SC', <django.utils.functional.__proxy__ object at 0x32ad310>), ('SD', <django.utils.functional.__proxy__ object at 0x32ad390>), ('TN', <django.utils.functional.__proxy__ object at 0x32ad410>), ('TX', <django.utils.functional.__proxy__ object at 0x32ad490>), ('UT', <django.utils.functional.__proxy__ object at 0x32ad510>), ('VA', <django.utils.functional.__proxy__ object at 0x32ad610>), ('VT', <django.utils.functional.__proxy__ object at 0x32ad590>), ('WA', <django.utils.functional.__proxy__ object at 0x32ad690>), ('WI', <django.utils.functional.__proxy__ object at 0x32ad790>), ('WV', <django.utils.functional.__proxy__ object at 0x32ad710>), ('WY', <django.utils.functional.__proxy__ object at 0x32ad810>))"

    Encapsulate a function call and act as a proxy for methods that are called on the result of that function. The function is not evaluated until one of the methods on the result is called.

Could you look into this? You can generate the documentation by install the packages in docs/requirements.txt and running make html in the docs directory. Thanks.

@PaulSD
Copy link
Contributor Author

PaulSD commented Dec 31, 2016

Bleh. I think we're hitting sphinx-doc/sphinx#857

I've copied the documentation from us_states.py to us.rst to work around it.

Commit 4369237 introduced lazy translations for US state names.
However, US_STATES, STATE_CHOICES, and USPS_CHOICES are sorted at import
time based on the translated state names.  This causes two problems:
1) If localflavor.us is imported during Django initialization, then an
   exception will be thrown because translations cannot be made during
   Django initialization.
   (See #203)
2) When the run-time translations are different than the import-time
   translations, US_STATES, STATE_CHOICES, and USPS_CHOICES are not
   re-sorted based on the run-time translations, so those lists may be
   mis-sorted.

This commit corrects these problems by lazily sorting US_STATES,
STATE_CHOICES, and USPS_CHOICES.

Unfortunately, while this works as expected in most cases, the lazy
evaluation may not work as expected in every case due to internal tuple
optimizations in the Python interpreter.  For example,
`US_STATES + ('XX', _('Select a State'))` works as expected, but
`('XX', _('Select a State')) + US_STATES` throws
'TypeError: can only concatenate tuple (not "__proxy__") to tuple'
because Python concatenates the tuples entirely in C code without making
any Python code calls to the US_STATES object, which prevents US_STATES
from being lazily generated before the concatenation is processed.  To
work around this, you can use a slice index to force a Python code
call on the object before any other operations are processed:
`('XX', _('Select a State')) + US_STATES[:]`
@benkonrath benkonrath merged commit 96263d7 into django:master Jan 3, 2017
@benkonrath
Copy link
Member

Thanks!

@PaulSD PaulSD deleted the lazy_init branch January 3, 2017 18:40
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants