Return to Answer

Take a look at PEP 8, which is the style guide for Python. Two obvious ways in which your code could be considered "un-Pythonic":

• Docstrings should live inside the function definition, not directly before them. This is covered further in PEP 257:

  A docstring is a string literal that occurs as the first statement in a module, function, class, or method definition.

  Single-line docstrings also don’t need separate lines for their opening and closing quotes; it can all go together.

• The Python convention for function names is lowercase with underscores, not mixed case. There’s also a tendency to use long, descriptive names (e.g. attributes instead of att), which would make your code easier to read.

The parse function declares the global variables tree and root, but this isn’t a good way to do it: if you try to parse two files in a row, then what happens? Um... The fact that this is ambiguous or potentially confusing means you should probably do something different.

I’d suggest turning this into a new class, with tree and root attributes. This allows you to work with multiple files in the same session. Something like this should get you started:

 class MentorTree(object):
 def __init__(self, filename):
 self.tree = ET.parse(filename)
 self.root = self.tree.getroot()

Most of the functions use if and for loops. It all works, but you can use list comprehensions and data structures to make the code more compact, and less nested. (Lots of nested constructions aren’t necessarily a bad thing, but I prefer to avoid them when I don’t need them.)

For example, in getNumGrids(), you can get the length of the iterator with a single list comprehension:

 def get_grid_count(self):
 ""Return the number of 'volume_grid' elements."""
 return sum(1 for _ in self.root.iter('volume_grid'))

Similarly for getGridNames():

 def get_grid_names(self):
 """Returns a list of the 'volume_grid' name attributes."""
 attributes = [grid.attrib for grid in root.iter('volume_grid')]
 return [n.get('name') for n in attributes]
Lots of other functions can be modified this way.

Where you have long strings of if statements of the form X != 'NC', you can clean then up with something like a dictionary or list. For example, here’s setSolverType():

 def set_solver_type(method, equations, num_time_steps, steady_state, time_step, cfl):
 """
 Change the solver element attributes.
 Attribute is not updated if the argument is 'None'.
 """
 element_attributes = [
 'method', 'equations', 'num_time_steps', 'steady_state', 'time_step', 'cfl'
 ]
 for sol in root.iter('solver'):
 for attribute in element_attributes:
 if eval(attribute) is not None:
 sol.set(attribute, eval(attribute))

Horrible abuse of eval(), but there you are. Note that I replaced 'NC' by None.

Take a look at PEP 8, which is the style guide for Python. Two obvious ways in which your code could be considered "un-Pythonic":

• Docstrings should live inside the function definition, not directly before them. This is covered further in PEP 257:

  A docstring is a string literal that occurs as the first statement in a module, function, class, or method definition.

  Single-line docstrings also don’t need separate lines for their opening and closing quotes; it can all go together.

• The Python convention for function names is lowercase with underscores, not mixed case. There’s also a tendency to use long, descriptive names (e.g. attributes instead of att), which would make your code easier to read.

The parse function declares the global variables tree and root, but this isn’t a good way to do it: if you try to parse two files in a row, then what happens? Um... The fact that this is ambiguous or potentially confusing means you should probably do something different.

I’d suggest turning this into a new class, with tree and root attributes. This allows you to work with multiple files in the same session. Something like this should get you started:

 class MentorTree(object):
 def __init__(self, filename):
 self.tree = ET.parse(filename)
 self.root = self.tree.getroot()

Most of the functions use if and for loops. It all works, but you can use list comprehensions and data structures to make the code more compact, and less nested. (Lots of nested constructions aren’t necessarily a bad thing, but I prefer to avoid them when I don’t need them.)

For example, in getNumGrids(), you can get the length of the iterator with a single list comprehension:

 def get_grid_count(self):
 """Return the number of 'volume_grid' elements."""
 return sum(1 for _ in self.root.iter('volume_grid'))

Similarly for getGridNames():

 def get_grid_names(self):
 """Returns a list of the 'volume_grid' name attributes."""
 attributes = [grid.attrib for grid in root.iter('volume_grid')]
 return [n.get('name') for n in attributes]
Lots of other functions can be modified this way.

Where you have long strings of if statements of the form X != 'NC', you can clean then up with something like a dictionary or list. For example, here’s setSolverType():

 def set_solver_type(method, equations, num_time_steps, steady_state, time_step, cfl):
 """
 Change the solver element attributes.
 Attribute is not updated if the argument is 'None'.
 """
 element_attributes = [
 'method', 'equations', 'num_time_steps', 'steady_state', 'time_step', 'cfl'
 ]
 for sol in root.iter('solver'):
 for attribute in element_attributes:
 if eval(attribute) is not None:
 sol.set(attribute, eval(attribute))

Horrible abuse of eval(), but there you are. Note that I replaced 'NC' by None.