-
-
Notifications
You must be signed in to change notification settings - Fork 685
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
Utilize __init_subclass__ instead of inspect on Locale mapping to improve type checking #920
Conversation
Codecov Report
@@ Coverage Diff @@
## master #920 +/- ##
==========================================
- Coverage 99.74% 99.74% -0.01%
==========================================
Files 10 10
Lines 1937 1934 -3
Branches 313 312 -1
==========================================
- Hits 1932 1929 -3
Misses 4 4
Partials 1 1
Continue to review full report at Codecov.
|
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.
One comment from me, otherwise, I don't see anything wrong from a first glance.
Will take a better look later today.
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.
LGTM but had a small comment
@@ -121,6 +123,13 @@ class Locale: | |||
|
|||
_month_name_to_ordinal: Optional[Dict[str, int]] | |||
|
|||
def __init_subclass__(cls, **kwargs: Any) -> None: | |||
for locale_name in cls.names: | |||
if locale_name in _locale_map: |
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.
Should we maintain the locale_name.lower()
like in the old code?
Pull Request Checklist
tox
ormake test
to find out!).tox -e lint
ormake lint
to find out!).master
branch.Description of Changes
Python 3.6 introduced
__init_subclass__
that invoked when any subclass is declared. It can replace behavior ofarrow.locales._map_locales()
.Additional feature
__init_subclass__
invoked on every subclassing -> Subclass ofLocale
outside ofarrow
package also registered onarrow.locales._locale_map
.Example