ISO code registry #248
ISO code registry #248
Conversation
setup.py
Outdated
| @@ -24,7 +24,7 @@ def read_relative_file(filename): | |||
| 'pyCalverter', | |||
| 'setuptools>=1.0', | |||
| ] | |||
| version = '2.4.0.dev0' | |||
| version = '2.4.0.1' | |||
There was a problem hiding this comment.
Yeah, right. I'll delete it.
|
|
||
|
|
||
| @iso_register('AT') |
|
|
||
|
|
||
| @iso_register('DE') |
There was a problem hiding this comment.
Everywhere that I see ISO codes mentioned they are upper case. I thought it's a standard
There was a problem hiding this comment.
ISO standard de_DE, where DE is Germany (country) and de is German (language)
country codes always upper case, and language codes always lower case
There was a problem hiding this comment.
Yes but because we are speaking only of countries here, we don't need the language code.
There was a problem hiding this comment.
Forgot to mention, the ISO standard (ISO-3166-1) have 3 options, ALPHA-2 (DE), ALPHA-3 (DEU), and Numerical (276), regional codes are ISO-3166-2 and 3166-3
There was a problem hiding this comment.
Since it is valid to combine any language with any country code, and i18n is handled differently and have little to nothing with looking up in calendars, lower case language code can safely be ignored.
Though it should be possible to look up ISO codes for each calendar with all three standards (ALPHA-2, ALPHA-3, Numerical)
There was a problem hiding this comment.
I propose to use two letter codes (ALPHA-2) for countries and specific ISO-3166-2: 1 (e.g. BO-L), 2 (e.g.DE-NW) or 3 letters (e.g. GB-WOR). And I suggest to keep in documentation list of supported regions along with their iso codes.
Current implementation only requires to have codes in format XX or XX-YY ((\w)+(-\w+)?)
|
Really good, do you mind fixing the flake8 issues? Mainly unused import in the registry. |
| class Austria(WesternCalendar, ChristianMixin): | ||
| "Austria" | ||
| name = 'Austria' |
There was a problem hiding this comment.
Do you mind creating a name property that return the docstring rather than changing all calendar to remove the docstring and adding a name property?
@property
def name(self):
return self.__doc__There was a problem hiding this comment.
I think class variable is a better place for things you want to use in your code rather than calling __doc__. Docstrings are for comments and docs. Besides, add a character and everything breaks.
There was a problem hiding this comment.
For me name is needed as I have helper class implemented in my Django app to display translated form choices and I use name attribute for translations. I could do without name, only with ISO code, but I think it's good idea to have such a variable on every Calendar instance.
you mean: |
workalendar/registry.py
Outdated
|
|
||
|
|
||
| # right now only european countries are supported in the ISO registry | ||
| from workalendar.europe import * # nopep8 |
There was a problem hiding this comment.
Since we are using flake8, I believe it should be # NoQA instead of # nopep8
|
For the line too long errors, we could probably accept longer lines. See: https://github.com/Kinto/kinto/blob/master/tox.ini#L46-L47 There are still some unused imports: https://travis-ci.org/novafloss/workalendar/jobs/273238912#L548-L567 |
|
I fixed for the warnings about imports and line lengths. |
|
For some reasons, travis refuses to install the Python 3.3 & Python 3.5 interpreter, which causes those tests to fail. |
|
👍 |
|
@brunobord rebased. travis green :) |
|
right. I'll merge this into master, and will try to schedule similar work on the other continents. there will probably be a bit of work on the documentation, in order to make sure that future contributions will follow the pattern-to-be. |
I needed a feature to load calendars based on countries and regions. I implemented it based on ISO codes. Such an enhancement was mentioned in #13.
I used decorators. This provides easily understandable solution, I think. Initially I started by using metaclasses, but it caused me problems with circular imports.