Hopefully fixing the last of Sarah's required changes for PR 41

generator/code_files/ValidationFiles.py

@@ -47,7 +47,7 @@
 
 
 class ValidationFiles():
-    """Class for all validation-related files   TODO Is it? """
+    """Class for all validation-related files"""
 
     def __init__(self, lib_object, verbose=False):
         # members from object

generator/tests/test_binding_code/run_bindings_tests.py

@@ -1,4 +1,28 @@
 #!/usr/bin/env python
+"""
+Generating the binding files creates a number of files for each language,
+as well as swig and local specific files.
+
+Every language will generate a file to downcast extensions, namespaces and
+specific classes.
+
+e.g. generating csharp bindings for package 'spatial' will result in files:
+
+bindings
+    csharp
+        local-downcast-extension-spatial.i
+        local-downcast-namespaces-spatial.i
+        local-packages-spatial.i
+    swig
+        spatial-packages.h
+        spatial-packages.i
+
+Certain languages will also require .cpp files.
+
+Testing these involves creating each of these specific files for a
+language/package combination and having distinct functions to create and
+compare the files.
+"""
 
 import os
 import sys
@@ -10,12 +34,14 @@
 
 import test_functions
 
+
 ##############################################################################
 # Set up variables
 fails = []
 not_tested = []
 
 
+
 ##############################################################################
 # Specific generation functions
 
@@ -55,7 +81,7 @@ def generate_bindings_downcast_ext(filename, binding):
 
 def generate_bindings_downcast_ns(filename, binding):
     """
-    Generate a downcast namespace.   TODO such as?
+    Generate a downcast namespace.
     """
     all_files = set_up(filename, binding)
     all_files.write_downcast_namespace()
@@ -64,7 +90,7 @@ def generate_bindings_downcast_ns(filename, binding):
 
 def generate_bindings_downcast_pkgs(filename, binding, local):
     """
-    Generate a downcast package. TODO such as?
+    Generate a downcast package.
 
     :param filename:
     :param binding:
@@ -82,7 +108,7 @@ def generate_bindings_downcast_pkgs(filename, binding, local):
 
 def generate_bindings_downcast_plugins(filename, binding):
     """
-    Generate a downcast plugin.  TODO such as?
+    Generate a downcast plugin.
     """
     all_files = set_up(filename, binding)
     all_files.write_downcast_plugins()
@@ -106,16 +132,6 @@ def compare_code(name, binding, ext):
                                         not_tested)
 
 
-def compare_ext_headers(class_name):
-    """
-    TODO This function doesn't appear to be used anywhere.
-    """
-    correct_file = os.path.normpath('./test-extension/{0}.h'.format(class_name))
-    temp_file = os.path.normpath('./temp/{0}.h'.format(class_name))
-    return test_functions.compare_files(correct_file, temp_file, fails,
-                                        not_tested)
-
-
 #############################################################################
 # Specific test functions
 
@@ -138,35 +154,9 @@ def run_ns_test(name, binding, ext):
     return fail
 
 
-# TODO I'm not sure what these different test functions are testing, exactly.
-"""
-Generating the binding files creates a number of files for each language,
-as well as swig and local specific files.
-
-Every language will generate a file to downcast extensions, namespaces and 
-specific classes. 
-
-e.g. generating csharp bindings for package 'spatial' will result in files:
-
-bindings
-    csharp
-        local-downcast-extension-spatial.i
-        local-downcast-namespaces-spatial.i
-        local-packages-spatial.i
-    swig
-        spatial-packages.h
-        spatial-packages.i
-
-Certain languages will also require .cpp files.
-
-Testing these involves creating each of these specific files for a 
-language/package combination and having distinct functions to create and
-compare the files. 
-"""
-
 def run_test(name, binding, ext):
     """
-    Run a test. TODO Sarah please expand...
+    Run a test.
 
     :param name: package name
     :param binding: the required binding, e.g. 'csharp', 'java', etc.
@@ -185,7 +175,7 @@ def run_test(name, binding, ext):
 
 def run_pkgs_test(name, binding, ext):
     """
-    Run a pkgs test. TODO Sarah please expand...
+    Run a pkgs test.
 
     :param name: package name
     :param binding: the required binding, e.g. 'csharp', 'java', etc.
@@ -207,12 +197,12 @@ def run_pkgs_test(name, binding, ext):
 
 def run_local_test(name, binding, ext, local):
     """
-    Run a local test. TODO Sarah please expand...
+    Run a local test.
 
     :param name: package name
     :param binding: the required binding, e.g. 'csharp', 'java', etc.
     :param ext: the file extension
-    :param local: TODO Sarah please expand...
+    :param local:
     :return: 0 on success or missing file, 1 on failure.
     """
     filename = test_functions.set_up_test(name, 'local', binding)
@@ -228,7 +218,7 @@ def run_local_test(name, binding, ext, local):
 
 def run_plugin_test(name, binding, ext):
     """
-    Run a plugin test. TODO Sarah please expand...
+    Run a plugin test.
 
     :param name: package name
     :param binding: the required binding, e.g. 'csharp', 'java', etc.
@@ -245,7 +235,7 @@ def run_plugin_test(name, binding, ext):
 
 def run_swig_test(name, binding, ext):
     """
-    Run a SWIG test. TODO Sarah please expand...
+    Run a SWIG test.
 
     :param name: package name
     :param binding: the required binding, e.g. 'csharp', 'java', etc.

generator/tests/test_cmake_code/run_cmake_tests.py

@@ -96,25 +96,16 @@ def compare_files(correct_file, temp_file):
                                         not_tested)
 
 
-def compare_cmake(name, is_src=False):
+def compare_cmake(name):  # , is_src=False):
     """
     Do a string comparison of the contents of two files.
 
-    TODO: the two cases checked by this function are identical!
-    Presumably a mistake?
-
     :param name: TODO
-    :param is_src: TODO
     :return: 0 on success, or file not present; 1 on failure.
     """
-    if is_src:
-        correct_file = os.path.normpath('./test-cmake/{0}-package.cmake'.
-                                        format(name))
-        temp_file = os.path.normpath('./temp/{0}-package.cmake'.format(name))
-    else:
-        correct_file = os.path.normpath('./test-cmake/{0}-package.cmake'.
-                                        format(name))
-        temp_file = os.path.normpath('./temp/{0}-package.cmake'.format(name))
+    correct_file = os.path.normpath('./test-cmake/{0}-package.cmake'.
+                                    format(name))
+    temp_file = os.path.normpath('./temp/{0}-package.cmake'.format(name))
     return compare_files(correct_file, temp_file)
 
 
@@ -180,7 +171,7 @@ def run_cmake_test(name):
     filename = test_functions.set_up_test(name, 'CMake')
     generate_cmake_files(filename)
     fail = compare_cmake(name)
-    fail += compare_cmake(name, True)
+    # fail += compare_cmake(name, True)
     print('')
     return fail
 

generator/tests/test_cpp_code/run_cpp_tests.py

@@ -38,9 +38,10 @@ def generate_new_cpp_header(filename, num):
     Generate cpp files.
 
     :param filename: name of XML file to parse.
-    :param num: TODO seems to be index of item in list which is the value
+    :param num: index of item in list which is the value
         (in the big dictionary structure) for the key 'baseElements'.
-        But I don't fully understand this.
+        It is the index so that we can do code for just one class and
+        not whole of the object returned from ParseXML
     :return: nothing
     """
     ob = common_setup(filename)
@@ -238,7 +239,8 @@ def run_plug_test(name, class_name, test_case, num):
     :param name: which set of tests this relates to, e.g. 'qual'
     :param class_name: C++ class name/start of .h and .cpp file names, e.g. 'QualModelPlugin'
     :param test_case: brief description, e.g. 'basic plugin'
-    :param num: plugin #. TODO I'm not too sure about this. Please can we have an example?
+    :param num: plugin #. It's just the index in the list of plugins that are
+                 part of large dict object form ParseXML
     :return: number of failed tests.
     """
     filename = test_functions.set_up_test(name, class_name, test_case)
@@ -731,8 +733,11 @@ def main():
         test_case = 'sidref with multiple targets'
         fail += run_valid_test(name, class_name, test_case)
 
-        # TODO Sarah, I'm guessing maybe the next few tests are iterating over something?
-        # Because it's not clear; all these "diff spacing" tests look identical.
+        # These next tests are testing the spacing of the element content
+        # eg.
+        # element="ThingA, ThingB"
+        # element=" ThingA, ThingB"
+        # element="ThingA,ThingB"
 
         name = 'test_sidrefs_1'
         class_name = 'RefsSBMLError'

generator/tests/test_examples/run_examples_tests.py

@@ -72,7 +72,8 @@ def generate_xml(filename):
 
 def generate_xml_fails(filename):
     """
-    Generate a failure example?????? TODO unsure
+    Generate a failure example (creates an xml
+    file that would fail validation)
 
     :param filename: XML file to parse
     :return: package name, e.g. 'dyn'
@@ -94,7 +95,7 @@ def generate_some_xml_fails(filename, start, stop, number=-1):
     :param number: number of files to write? TODO not sure.
     :return: package name, e.g. 'dyn'
 
-    TODO Sorry Sarah I'm a bit confused by this one!
+    Sarah: "This was a work in progress and is not included in overall test run"
     """
     ob = set_up(filename)
     all_files = ValidationXMLFiles.ValidationXMLFiles(ob)

generator/tests/test_other_library/run_other_library_tests.py

@@ -59,7 +59,8 @@ def generate_new_cpp_header(filename, num):
     Generate new C++ header and code files.
 
     :param filename: the XML file to parse.
-    :param num: TODO not too sure about this - it's a list index.
+    :param num: index allowing code to generate for just one class rather
+                than all - it's a list index.
     :return: nothing
     """
     ob = common_set_up(filename)
@@ -74,11 +75,17 @@ def generate_new_cpp_header(filename, num):
 
 def generate_templates(filename):
     """
-    TODO not sure what this does. Not sure what ValidationFiles class is for.
-    And what do we mean by 'templates' here?
+    Several of the files for other libraries are generated using
+    templates with appropriate names etc added. See
+    generator/code_files/templates.
+    This function tests these
 
     :param filename: the XML file to parse.
     :return: nothing
+
+    Validation class causes the errors for things to be enumerated and strings
+    added. These will be needed by templates so with testing we have to
+    create them first
     """
     ob = common_set_up(filename)
     gv.populate_error_list(ob)
@@ -93,7 +100,8 @@ def generate_templates(filename):
 
 def generate_common_templates(filename):
     """
-    Write the 'common' files. TODO need more explanation.
+    Write the 'common' files. These are the files that will be part of
+    the common directory as they are used by all files in library.
 
     :param filename: the XML file to parse.
     :return: nothing

sphinx-docs/conf.py

@@ -23,7 +23,7 @@
 project = 'generator'
 copyright = '2020, Sarah Keating et al'
 author = 'Sarah M. Keating, Frank T. Bergmann, Michael Hucka,' \
-         'Brett G. Olivier, Lucian P. Smith'
+         'Brett G. Olivier, Lucian P. Smith, Matthew S. Gillman'
 
 
 # -- General configuration ---------------------------------------------------