-
Notifications
You must be signed in to change notification settings - Fork 292
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
Added full names and numerical codes for Swedish counties #117
base: master
Are you sure you want to change the base?
Conversation
I have added the following dictionaries and functions to se_counties: NUMERICAL_COUNTY_CODES contains the numerical two-digit couty codes which has more or less replaced the alphabetical codes nowadays. Also, these numerical codes is always the leading digits in the codes for municipailites and smaller subdivisions, which can be beneficial. FULL_COUNTY_NAMES contains the full names of each county. The current labels given in SE_COUNTIES is good when it is obvioius to the user that he/she is selecting a county, but they are not identical to the full county name. Also, in Swedish the county names differ somewhat, e.g. "Skåne län" as opposed to the more generic "Stockholms län" (ending with genitive case s), which the Swedish i18n should honor. numerical_county_code() and full_county_name() are simply accessor methods for the above dictionaries, returning the corresponding values given a alphabetical county code. For backwards compatibility I have chosen to keep both the alphabetical codes and the shortened names (lacking the "county"-part) in the original SE_COUNTIES list, although I guess one could argue numerical codes and full names would be a better option.
The decorator adds get_FIELDNAME_numerical_code() and get_FIELDNAME_full_name() methods to django models for all FIELDNAME values supplied in the field_names tuple. Ideally this can be used on all model fields that uses COUNTY_CHOICES for the choices argument. @county_decorator(field_names('field',)) class MyClass: field = models.CharField(choices=se_counties=COUNTY_CHOICES)
} | ||
|
||
|
||
def numerical_county_code(county_code): |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Both of those helpers won't be necessary as they just wrap dict functionality, which seems moot. Please remove.
I think the decorator is a nifty feature but frankly out of scope of localflavor. Instead I'd much rather see this implemented as a model field class that uses the |
Ah, regarding Transifex please request access to the team on the platform. We leave the management of language teams to the appropriate language coordinators. |
I understand, I'll remove the helper functions and the decorator. Would you say a field based implementation using |
@dessibelle Yeah, a field would work as we ship with plenty of them. I just want to keep the API we publish to the users as common as possible. |
@jezdez Sure, I totally understand, and I value your clarifications. I had basically just solved the problem for myself, without too much knowledge of the API in a broader sense. |
…sors for full county name and numerical codes. Also renamed `FULL_COUNTY_NAMES` and `NUMERICAL_COUNTY_CODES` in `se_counties.py` into `NUMERICAL_COUNTY_CODE_CHOICES` and `FULL_COUNTY_NAME_CHOICES` respectively, as well as converted them both to tuple lists rather than dicts in order for them to be supplied as the `chocies` argument for the field.
I have added the following alternative choice options to
se_counties.py
:NUMERICAL_COUNTY_CODE_CHOICES
contains the numerical two-digit couty codes which has more or less replaced the alphabetical codes nowadays. Also, these numerical codes is always the leading digits in the codes for municipailites and smaller subdivisions, which can be beneficial.FULL_COUNTY_NAME_CHOICES
contains the full name of each county. The current labels given inSE_COUNTIES
is good when it is obvioius to the user that he/she is selecting a county, but they are not identical to the full county name. Also, in Swedish the county names differ somewhat, e.g. "Skåne län" as opposed to the more common "Stockholms län" (ending with genitive case s), which the Swedish i18n should honor.For backwards compatibility I have chosen to keep both the alphabetical codes and the shortened names (lacking the "county"-part) in the original
SE_COUNTIES
list, although I guess one could argue numerical codes and full names would be a better option.I also added a
SECountyField
field that contributes a few accessor methods to the model class, allowing the you to access numerical county codes and full county names for a given field like so:If I have understood the other pull requests correctly, localizations is managed through Transifex. I'd be glad to add Swedish localizations for the introduced changes if I could get access.