7
\$\begingroup\$

My first question relates to the first Python module that I published on Python. This is a very small and simple module, the package contains only one class. This is a port of a Perl module Locale::Wolowitz this is a module for managing translations, but uses the serialization format JSON or YAML. Below the code of my Python class, I will like to have an opinion on the code:

import glob
import re
import os.path
from collections import defaultdict


class Pylocwolowitz(object):
    '''Pylocwolitz is a very simple text localization system, meant to be used
by web applications (but can pretty much be used anywhere). Yes, another
localization system.'''

def __init__(self, path, format_deserializer='json'):

    if format_deserializer not in ('json', 'yaml'):
        raise ValueError('FormatNotSupported')

    self.path = path
    self.format_deserializer = format_deserializer
    self.locales = defaultdict(dict)
    self._find_file()

def _find_file(self):
    '''Find all json files'''
    listing = glob.glob(
        os.path.join(self.path, '*.' + self.format_deserializer))
    for infile in listing:
        self._make_loc(infile)

def _make_loc(self, infile):
    '''Store content of the file in a memory'''
    lang = os.path.basename(infile).split('.')[0]
    if self.format_deserializer == 'json':
        import json
        with open(infile) as fil:
            data = json.load(fil)
    else:
        import yaml
        with open(infile) as fil:
            data = yaml.load(fil)
    for key, value in data.items():
        if isinstance(value, dict):
            self.locales[key].update(value)
        else:
            self.locales[key].update({lang: value})

def loc(self, key, lang, values=None):
    '''Return the string key, translated to the requested language (if such
    a translation exists, otherwise no traslation occurs). Any other
    parameters passed to the method are injected to the placeholders in the
    string (if present).'''

    if self.locales[key].get(lang) is None:
        return key

    ret = self.locales[key][lang] if key in self.locales else key
    if values is None:
        return ret
    else:
        return ret % values

I get an error with Python 3.2 on travis.ci but the tests pass on Python 2.x and PyPy, if you have an idea let me know. I'm not sure, but I think as I read it was better to use format or template, rather than % with Python 3 is that I'm wrong.

\$\endgroup\$
5
\$\begingroup\$
import glob
import re
import os.path
from collections import defaultdict


class Pylocwolowitz(object):
    '''Pylocwolitz is a very simple text localization system, meant to be used
by web applications (but can pretty much be used anywhere). Yes, another
localization system.'''

def __init__(self, path, format_deserializer='json'):

Calling it format_deserializer suggests to me that it should be a function. Perhaps it should just be format?

    if format_deserializer not in ('json', 'yaml'):
        raise ValueError('FormatNotSupported')

    self.path = path
    self.format_deserializer = format_deserializer
    self.locales = defaultdict(dict)
    self._find_file()

def _find_file(self):
    '''Find all json files'''

You are finding files, find a file. Consider changing the name.

    listing = glob.glob(
        os.path.join(self.path, '*.' + self.format_deserializer))
    for infile in listing:

I'd do

 path = os.path.join(self.path, '*.' + self.format_deserializer)
 for infile in glob.glob(path):

Which I think comes out a bit cleaner

        self._make_loc(infile)

def _make_loc(self, infile):
    '''Store content of the file in a memory'''

Avoid abbreviations, I have no idea what loc is.

    lang = os.path.basename(infile).split('.')[0]

There is a splitext function which you might find useful here. You also don't ue this until way latter. Perhaps you should determine it later.

    if self.format_deserializer == 'json':
        import json
        with open(infile) as fil:
            data = json.load(fil)
    else:
        import yaml
        with open(infile) as fil:
            data = yaml.load(fil)

Instead, you could do

 deseralizer = __import__(self.format_deserializer)
 with open(infile) as fil:
    data = deserializer.load(fil)

That would avoid doing the same thing twice.

    for key, value in data.items():
        if isinstance(value, dict):
            self.locales[key].update(value)
        else:
            self.locales[key].update({lang: value})

def loc(self, key, lang, values=None):
    '''Return the string key, translated to the requested language (if such
    a translation exists, otherwise no traslation occurs). Any other
    parameters passed to the method are injected to the placeholders in the
    string (if present).'''

    if self.locales[key].get(lang) is None:
        return key

Do you actually want to skip formatting this case?

    ret = self.locales[key][lang] if key in self.locales else key

Why are you checking if key is in self.locales? It will be because the previous statement will have put it in regardless. And if it's not there, the first statement would have already returned. Also, its more pythonic to try and fetch it and catch the exception like:

   try:
       ret = self.locales[key][lang]
   except KeyError:
       ret =  key

That way you only lookup the dictionary once. But we can actually do even better by using get with a default

   ret = self.locales[key].get(lang, key)

That, I think, is a very slick approach.

    if values is None:
        return ret
    else:
        return ret % values

Using .format would probably be preferred

\$\endgroup\$
  • \$\begingroup\$ Hi, Thank you very much for your answer, I'm very happy, I will improve it. \$\endgroup\$ – Hobbestigrou Aug 11 '13 at 2:50

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.