Merging tags in two XML files with same attributes and different content

Question

From time to time I translate some Android apps: the developer sends me an XML file, e.g.

<string name="foo">Foo</string>
<string name="baz">Baz</string>

and I send him back an XML file where the content of each element has been translated, e.g.

<string name="foo">Translated foo</string>
<string name="baz">Translated baz</string>

The problem arises when the developer adds some new text element and sends me a new file to be translated, e.g.

<string name="foo">Foo</string>
<string name="bar">Bar</string>
<string name="baz">Baz</string>

Thus I want to merge the two files, adding a marker at the beginning of the new lines to be translated. Given the previous example, this would mean generating a file like

<string name="foo">Translated foo</string>
<string name="bar">***new***Bar</string>
<string name="baz">Translated baz</string>

---

Since I didn't find any pre-existing solution I decided to write a small Python script myself which so far seems to do the job:

"""Usage: merge_strings.py [-m <mrk>] [-o <file> [-i]] <old_xml> <new_xml>

Substitutes the content of any 'string' tag from <new_xml> with the
content of a 'string' tag from <old_xml> with the same 'name' attribute,
if present, otherwise prepends it with <mrk>.
By default the result is printed to stdout.

Note: This program assumes that no two 'string' tags in the same file
have the same 'name' attribute. Furthermore, 'string' tags with names
unique to <old_xml> are ignored.

Options:
    -h --help                 Show this screen.
    -m <mrk> --marker <mrk>   Marker for new strings [default: ***new***].
    -o <file>                 Print to <file> instead.
    -i --interactive          Check before overwriting <file>.
"""

from os.path import isfile
from sys import exit

from docopt import docopt
import lxml.etree as etree


def merge_strings(old, new, marker):
    """
    Merge in place synonymous strings from 'old' into 'new'.
    Ignores strings unique to 'old' and prepends strings unique to
    'new' with 'marker'.
    """
    for s in new.iterfind('//string'):
        name = s.attrib['name']
        t = old.find("//string[@name='" + name + "']")

        if t is not None:
            s.text = t.text
        else:
            s.text = marker + s.text

def check_overwrite(path):
    """
    Check if we want to overwrite 'path' and exit if not.
    Defaults to no.
    """
    print("About to overwrite:", path)
    choice = input("Continue? [y/N]")

    if choice.lower() != 'y':
        exit(0)

def print_to_file(tree, path, interactive=False):
    if interactive and isfile(path):
        check_overwrite(path)

    with open(path, mode='wb') as f:
        tree.write(f, pretty_print=True,
                      encoding='utf-8',
                      xml_declaration=True)

def print_to_stdout(tree):
    print(etree.tostring(tree, pretty_print=True,
                               encoding='utf-8',
                               xml_declaration=True).decode('utf-8'))


if __name__ == '__main__':
    args = docopt(__doc__)

    old_tree = etree.parse(args['<old_xml>'])
    new_tree = etree.parse(args['<new_xml>'])

    merge_strings(old_tree, new_tree, args['--marker'])

    if args['-o']:
        print_to_file(new_tree, args['-o'], args['--interactive'])
    else:
        print_to_stdout(new_tree)

Here is the obligatory example output:

$cat tests/old.xml 
<?xml version="1.0" encoding="utf-8"?>
<resources>
  <string name="foo">Translated foo</string>
  <string name="baz">Translated baz</string>
</resources>

$cat tests/new.xml 
<?xml version="1.0" encoding="utf-8"?>
<resources>
  <string name="foo">Foo</string>
  <string name="bar">Bar</string>
  <string name="baz">Baz</string>
</resources>

$python merge_strings.py old.xml new.xml                                                   
<?xml version='1.0' encoding='utf-8'?>
<resources>
  <string name="foo">Translated foo</string>
  <string name="bar">***new***Bar</string>
  <string name="baz">Translated baz</string>
</resources>

---

Remarks: This is only my second Python program and it is the first time I dabble with XML. As such, I would like to know if this code follows Python best practices and if it can be improved in terms of readability and/or performance.

Answer 1

First a little disclaimer: I haven't worked with either docopt or etree before. So I can't comment on the usage/correctness of this

Style reviews

Overall the code looks OK, but there are stuff comment upon:

• Process arguments at start – Instead of referring to args[whatever] throughout your main code part, I would check the different parameters at the start of your main block, and then set proper flags and variables to be used later on. This would also allow for proper handling if the arguments are missing were you could substitute some of them with default values
• No handling of -m (or -i) argument? – Your usage strings says that you could use either -m or --marker, and that it has a default of ***new***. Can't see the code providing this? (This would be clearer if you had implemented previous point)
• Not too keen on your naming – The names old, new, s and t are somewhat confusing. Maybe something along the lines of old_translation, new_source, native (or source?) and translation would be better?
• Decide on one of ' and " – You have a bit of both... (Just like I often do... ;-) )
• Avoid string concatenation – Instead of "//string[@name='" + name + "']", use "//string[@name='{}']".format(name). Note that I'm personally aiming for using single quotes in most strings, but if my new string is to have single quotes, you either need to escape this \' or switch around and use double quotes for that string
• No error handling – There is little or none error handling if any of the file operations fails, or arguments missing, and so on
• Mostly good docstrings – Even though they are lacking for the latter methods, and the style guidelines says to include a summary in the first line in multi-line docstrings
• Recommendation is two newlines before def – If you increase to two newlines, the methods will stand out a little more, and you fill follow the guidelines even more.

Performance issue

One option I believe could hurt you performance wise is the search in old for any translated resource. This will all the time do an \$O(n)\$ search, and thusly render your entire code for merging to be \$O(n^2)\$.

If you instead had made a dictionary of all the old strings, the search could be more like \$O(1)\$ (or some low cost for checking the dictionary) at cost of a prerun through old. This would render the merge process to be \$2\cdot O(n)\$, which is similar/counted as \$O(n)\$. Where \$n\$ is the number of strings to be translated.

In other words, if using a dictionary for the old strings, you would most likely get a much faster merge operation when the number of strings to be translated increases.

PS! As it is a little late in the night in my part of the world, I have not written any new code. Please tell if some of the comments are not understandable or you don't know how to correct it

Answer 2

I have two minor style/readability notes.

I think saying "defaults to no" for check_overwrite(path) is a little misleading. Python functions can have default values that get used if no value is supplied for particular parameters, so I misunderstood at first and thought you had removed a parameter that used to be there. Instead you could be clear that it only accepts 'y' and exits for all other input.

It's also more idiomatic to use if var is None than is not None when you can so I'd flip the test in merge_strings.

    if t is None:
        s.text = marker + s.text
    else:
        s.text = t.text

You could use the ternary operator here. Opinion varies on whether it's good form but it would cut down to one line:

    s.text = marker + s.text if t is None else t.text