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

modified normalize_locale and exists to handle various unexpected input #523

Merged
merged 3 commits into from
Sep 12, 2017

Conversation

suhojm
Copy link
Contributor

@suhojm suhojm commented Sep 9, 2017

made normalize_locale and exists to handle various unexpected input such as list or None just like currency, and added test cases in test_localedata.py
resolves: #521

made normalize_locale and exists to handle various unexpected input such as list or None
resolves: python-babel#521
name = name.strip().lower()
for locale_id in chain.from_iterable([_cache, locale_identifiers()]):
if name == locale_id.lower():
return locale_id



Choose a reason for hiding this comment

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

Line contains following spacing inconsistencies:

  • Trailing whitespaces.

SpaceConsistencyBear, severity NORMAL, section default.

The issue can be fixed by applying the following patch:

--- a/babel/localedata.py
+++ b/babel/localedata.py
@@ -47,7 +47,7 @@
     for locale_id in chain.from_iterable([_cache, locale_identifiers()]):
         if name == locale_id.lower():
             return locale_id
-    
+
 def exists(name):
     """Check whether locale data is available for the given locale.
 

@codecov-io
Copy link

codecov-io commented Sep 9, 2017

Codecov Report

Merging #523 into master will decrease coverage by 1.35%.
The diff coverage is 100%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #523      +/-   ##
==========================================
- Coverage   90.19%   88.84%   -1.36%     
==========================================
  Files          24       24              
  Lines        4020     4024       +4     
==========================================
- Hits         3626     3575      -51     
- Misses        394      449      +55
Impacted Files Coverage Δ
babel/localedata.py 94.82% <100%> (-0.71%) ⬇️
babel/_compat.py 53.48% <0%> (-46.52%) ⬇️
babel/localtime/_unix.py 11.53% <0%> (-20.52%) ⬇️
babel/localtime/_win32.py 59.25% <0%> (-3.71%) ⬇️
babel/localtime/__init__.py 62.5% <0%> (-2.5%) ⬇️
babel/messages/catalog.py 92.23% <0%> (-2.39%) ⬇️
babel/util.py 90.78% <0%> (-1.32%) ⬇️
babel/messages/extract.py 94.18% <0%> (-1.1%) ⬇️
babel/support.py 82.28% <0%> (-0.4%) ⬇️
... and 1 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update c7729cc...1992b10. Read the comment docs.

name = name.strip().lower()
for locale_id in chain.from_iterable([_cache, locale_identifiers()]):
if name == locale_id.lower():
return locale_id



Choose a reason for hiding this comment

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

Line contains following spacing inconsistencies:

  • Trailing whitespaces.

SpaceConsistencyBear, severity NORMAL, section default.

The issue can be fixed by applying the following patch:

--- a/babel/localedata.py
+++ b/babel/localedata.py
@@ -47,7 +47,7 @@
     for locale_id in chain.from_iterable([_cache, locale_identifiers()]):
         if name == locale_id.lower():
             return locale_id
-    
+
 def exists(name):
     """Check whether locale data is available for the given locale.
 

name = name.strip().lower()
for locale_id in chain.from_iterable([_cache, locale_identifiers()]):
if name == locale_id.lower():
return locale_id



Choose a reason for hiding this comment

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

Line contains following spacing inconsistencies:

  • Trailing whitespaces.

SpaceConsistencyBear, severity NORMAL, section default.

The issue can be fixed by applying the following patch:

--- a/babel/localedata.py
+++ b/babel/localedata.py
@@ -47,7 +47,7 @@
     for locale_id in chain.from_iterable([_cache, locale_identifiers()]):
         if name == locale_id.lower():
             return locale_id
-    
+
 def exists(name):
     """Check whether locale data is available for the given locale.
 

name = name.strip().lower()
for locale_id in chain.from_iterable([_cache, locale_identifiers()]):
if name == locale_id.lower():
return locale_id



Choose a reason for hiding this comment

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

Line contains following spacing inconsistencies:

  • Trailing whitespaces.

SpaceConsistencyBear, severity NORMAL, section default.

The issue can be fixed by applying the following patch:

--- a/babel/localedata.py
+++ b/babel/localedata.py
@@ -47,7 +47,7 @@
     for locale_id in chain.from_iterable([_cache, locale_identifiers()]):
         if name == locale_id.lower():
             return locale_id
-    
+
 def exists(name):
     """Check whether locale data is available for the given locale.
 

name = name.strip().lower()
for locale_id in chain.from_iterable([_cache, locale_identifiers()]):
if name == locale_id.lower():
return locale_id



Choose a reason for hiding this comment

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

Line contains following spacing inconsistencies:

  • Trailing whitespaces.

SpaceConsistencyBear, severity NORMAL, section default.

The issue can be fixed by applying the following patch:

--- a/babel/localedata.py
+++ b/babel/localedata.py
@@ -47,7 +47,7 @@
     for locale_id in chain.from_iterable([_cache, locale_identifiers()]):
         if name == locale_id.lower():
             return locale_id
-    
+
 def exists(name):
     """Check whether locale data is available for the given locale.
 

@@ -41,6 +41,8 @@ def normalize_locale(name):
Returns the normalized locale ID string or `None` if the ID is not
recognized.
"""
if not name or not isinstance(name, string_types):
Copy link
Member

Choose a reason for hiding this comment

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

It would probably be better and/or more Pythonic to just cast to the text type here, instead of strictly requiring a string type.

@@ -54,6 +56,8 @@ def exists(name):

:param name: the locale identifier string
"""
if not name or not isinstance(name, string_types):
Copy link
Member

Choose a reason for hiding this comment

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

Same here: It would probably be better and/or more Pythonic to just cast to the text type here, instead of strictly requiring a string type.

Copy link
Contributor

Choose a reason for hiding this comment

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

Perhaps that's functionality that could be implemented at a later date?

Using isinstance() would make it match the currency functions, e.g., https://github.com/python-babel/babel/blob/master/babel/numbers.py#L82.

I'm also not sure whether accepting an object for which str(obj) returns a locale should be supported long-term?

Copy link
Member

Choose a reason for hiding this comment

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

I suppose you're correct. Didn't remember the currency functions already acted like this anyway.

@akx akx merged commit 1903a2b into python-babel:master Sep 12, 2017
This pull request was closed.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Validation / normalization of locales and currencies differ in behavior
6 participants