2
\$\begingroup\$

I made this program to rename variables in json files, part of the functions rely on recursivity which i'm not confortable with so I would like some feedback on readability and performance.

import configparser

def seek_in_array(json_array, conversions):
    """seeks all data in a json array"""
    converted = []
    for it_list in range(len(json_array)):
        if isinstance(json_array[it_list], dict):
            converted.append(seek_and_convert(json_array[it_list], conversions))
        elif isinstance(json_array[it_list], list):
            converted.append(seek_in_array(json_array[it_list], conversions))
        else:
            converted.append(json_array[it_list])
    return(converted)

def seek_and_convert(json_file, conversions):
    """seeks all data in a json file an converts it"""
    converted = {}
    for key in json_file.keys():
        if conversions.has_option('CONVERSIONS', key):
            option = conversions.get('CONVERSIONS', key)
        else:
            option = key
        if isinstance(json_file[key], dict):
            converted[option] = seek_and_convert(json_file[key], conversions)
        elif isinstance(json_file[key], list):
            converted[option] = seek_in_array(json_file[key], conversions)
        else:
            converted[option] = json_file[key]
    return(converted)

def load_conversion_table():
    """loads the configfile containing the conversions to be made"""
    conversions = configparser.ConfigParser()
    conversions.read('conversion_table.ini')
    return conversions

def main():
    conversions = load_conversion_table()
    file = {
              '_id': '006480206',

              'change1': 'HEY'
              'A_THING': 'HEY'
              'an_array': [
                {
                  'A_THING_in_the_array': '00648020600017',
                  'oh_no_,_change_that':'O',
                  'plz_dont_change_me': '12',
                  'listenning_to_disco_music': True,
                  'le_list': {
                    'le_variable': 'baguette !',
                    'le_change': 42,
                  },
                  "that's a trap": {},
                  "change_it_anyway": {},
                  'another_array': [
                    {
                      'change1': 'HANS !',
                      'change2': 'JA !?'
                    },
                    {
                      'nei': 'yas'
                      'change1': 'the very test',
                    },
                    {
                      'change1': 'ran out of idea',
                      'yas_yas': 'nomnomnom'
                    },
                    {
                      'OOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOH': 'such long name'
                    }
                  ]
                }
              ]
            }
    print(seek_and_convert(file, conversions))

if __name__ == "__main__":
    main()

Here is the config file you'll need to run the program, put both file in the same folder.
conversion_table.ini:

[CONVERSIONS]
change1 = success1
oh_no_,_change_that = oh_no_,_success_that
plz_dont_change_me = plz_dont_success_me
le_change = le_success
change_it_anyway = success_it_anyway
change2 = success2
\$\endgroup\$
1
\$\begingroup\$

looping

Don't loop over indices. the iterattion used by Python is of such simplicity and elegance, and there are some really useful builtin helper functions, that looping over the index (or keys for a dict) is hardly ever necessary

I suggest you check out the excellent 'Looping like a Pro' talk by David Baumgold

def seek_and_convert(json_array, conversions):
    """seeks all data in a json array"""
    converted = []
    for item in json_array:
        if isinstance(item, dict):
            converted.append(seek_and_convert(item, conversions))
        elif isinstance(json_array[it_list], list):
            converted.append(seek_in_array(item, conversions))
        else:
            converted.append(item)
    return(converted)

the same for the dict

def seek_in_array(json_file, conversions):
    """seeks all data in a json file an converts it"""
    converted = {}
    for key, item in json_file.items():
        if conversions.has_option('CONVERSIONS', key):
            option = conversions.get('CONVERSIONS', key)
        else:
            option = key
        if isinstance(item, dict):
            converted[option] = seek_and_convert(item, conversions)
        elif isinstance(item, list):
            converted[option] = seek_in_array(item, conversions)
        else:
            converted[option] = item
    return(converted)

conversions.get

ConfigParser.get has a fallback argument, so instead of

if conversions.has_option('CONVERSIONS', key):
    option = conversions.get('CONVERSIONS', key)
else:
    option = key

This, together with the use of keyword arguments, can be converted to:

option = conversions.get(section='CONVERSIONS', option=key, fallback=key)

For testing and future expansio, it would also make more sense to pass the conversions on as a dict, instead as a ConfigParser object. You use it as a dict anyway. Together with Hoist the IO, you get something like:

with StringIO(config_str) as config_file:
    config_parser = ConfigParser()
    config_parser.read_file(config_file)
    conversions = dict(config_parser.items(section="CONVERSIONS"))

In your real app, you can use with open(file, "r") as config_file instead of the StringIO

Then further on, all of this:

if conversions.has_option('CONVERSIONS', key):
    option = conversions.get('CONVERSIONS', key)
else:
    option = key

becomes:

option = conversions.get(key, key)

In contrast to your current load_conversion_table method, where you've hardcoded the filename, return a ConfigParser, and need to specify the section each time.

generators

Instead of keeping an intermediary dict or list, you can also yield the items:

def convert_dict(json_file, conversions):
    """seeks all data in a json file an converts it"""
    for key, item in json_file.items():
        option = conversions.get(key, key)
        if isinstance(item, dict):
            yield option, dict(seek_and_convert(item, conversions))
        elif isinstance(item, list):
            yield option, list(seek_in_array(item, conversions))
        else:
            yield option, item

dispatching

Instead of a chain of isinstance conditions, where depending on the type, a certain method is called with the same arguments, you can use the fact that functions are first-class citizens in Python and build a dict of parsers:

from collections import defaultdict


def fallback_parser():
    def parser(item, conversion):
        """parser that just returns the item, without conversion"""
        return item
    return parser


PARSERS = defaultdict(fallback_parser)
PARSERS[dict] = lambda item, conversion: dict(convert_dict(item, conversion))
PARSERS[list] = lambda item, conversion: list(convert_array(item, conversion))
PARSERS

and the parser functions become as simple as:

def convert_dict(json_dict, conversions):
    for key, item in json_dict.items():
        option = conversions.get(key, key)
        yield option, PARSERS[type(item)](item, conversions)

def convert_array(json_array, conversions):
    for item in json_array:
        yield PARSERS[type(item)](item, conversions)

If, later, you want support for datetime or other types, etc, this can be added by simply writing the parser and extending the PARSERS dict.

functools.singledispatch

An alternative is functools.singledispatch

from functools import singledispatch
@singledispatch
def parser(item, conversions):
    """parser that just returns the item, without conversion"""
    return item

@parser.register
def _(json_dict: dict, conversions):
    result = {}
    for key, item in json_dict.items():
        option = conversions.get(key, key)
        result[option] = parser(item, conversions)
    return result

@parser.register 
def _(json_array: list, conversions):
    result = []
    for item in json_array:
        result.append(parser(item, conversions))
    return result

If you prefer the generator approach, you can do something like:

@singledispatch
def parser(item, conversions=None):
    """parser that just returns the item, without conversion"""
    return item


def convert_dict(json_dict: dict, conversions: dict = None):
    conversions = {} if conversions is None else conversions
    for key, item in json_dict.items():
        option = conversions.get(key, key)
        yield option, parser(item, conversions)


def convert_array(json_array: list, conversions: dict = None):
    for item in json_array:
        yield parser(item, conversions)


parser.register(
    dict, lambda item, conversions=None: dict(convert_dict(item, conversions))
)
parser.register(
    list, lambda item, conversions=None: list(convert_array(item, conversions))
)
\$\endgroup\$

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.